Commit 4cd87c41d1f7e47b66723891d884613dceeb81d8
1 parent
75162ac6
feat: task.execution_priority column + v_task_board display/edit column
Requested by user after today's incident retrospective: a manually-assignable execution-order hint across a project's tasks, shown as a new "执行顺序" column on v_task_board. Serial steps are whole numbers (1, 2, 3); parallel siblings of one step share the same integer part as decimals (2.1, 2.2). Purely advisory -- nothing in the scheduler/dispatch loop reads it, it's a human/Boss-Agent-facing planning aid, not an enforcement mechanism. - schema.py: task.execution_priority REAL (nullable), migrate_add_task_execution_priority (plain ADD COLUMN, no CHECK constraint). Two earlier CHECK-rebuild migrations that also touch task (migrate_relax_org_node_constraints, migrate_add_reviewing_planner_ output_status) now call it as a prerequisite and include the column in their own rebuilt task_new, same "depends on a later additive column" pattern migrate_relax_org_node_constraints already used for migrate_add_work_graph_fork_columns. - task_query.py: list_tasks/get_task expose it; new update_task_execution_priority (unlike update_task_goal_text, NOT gated to status='queued' -- it's a display hint, not a planning input, so it stays settable on in-flight/blocked tasks too). - backend_server.py: PUT /api/tasks/:id now accepts execution_priority independently of goal_text on the same endpoint. - frontend/app.js: new editable column on v_task_board, PUTs on change. Also adds docs/todo/append-run-node-id-collision-across-org-nodes-requirements.md, a third real finding from today's retrospective (task 911861fe: Coder and Validator's suggested node catalogs both converge on node_type="test_script", so their self-planned node_ids collide inside a shared merged work_graph_run -- PlanningFailedError, uncaught by _ROUTABLE_EXCEPTIONS, falls into the same generic execution_error bucket FR-7 already flagged). 813/813 src tests, 134/134 supervisor tests, frontend smoke passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
8 changed files
with
292 additions
and
16 deletions
docs/todo/append-run-node-id-collision-across-org-nodes-requirements.md
0 → 100644
| 1 | +# 追加规划的 node_id 跨 org_node 撞名——需求文档 | ||
| 2 | + | ||
| 3 | +**日期**: 2026-08-13 | ||
| 4 | +**性质**: 需求文档(BRD/PRD 层面),不含详细技术设计。喂给 graphlemon 自己实现的 Goal,本次会话 | ||
| 5 | +不直接改代码。 | ||
| 6 | +**状态**: 草案,待确认后作为 Goal 提交。 | ||
| 7 | + | ||
| 8 | +## 1. 背景 | ||
| 9 | + | ||
| 10 | +`work_graph_plan.py::plan_work_graph(..., append_to_run_id=...)`(Phase 2c 的合并 run 架构, | ||
| 11 | +Coder/Validator 各自往同一个共享 run 里追加自己规划的节点)在检测到新规划的 `node_id` 跟已有节点 | ||
| 12 | +撞名时会直接拒绝: | ||
| 13 | + | ||
| 14 | +```python | ||
| 15 | +collision = existing_ids_set & set(new_node_ids) | ||
| 16 | +if collision: | ||
| 17 | + raise PlanningFailedError( | ||
| 18 | + "append_to_run_id={} node_id collision with existing nodes: {}".format(append_to_run_id, collision) | ||
| 19 | + ) | ||
| 20 | +``` | ||
| 21 | + | ||
| 22 | +调用方(`plan_work_graph`)会把已有节点 id 列表写进 prompt,明确要求"不要重复用这些 id",但今天 | ||
| 23 | +(2026-08-13)真实任务 `911861fe` 撞上了这个拒绝:Validator 自己规划节点时选了 `n_test_script` | ||
| 24 | +这个 id,而这个 id 在同一个 run 里已经被 Coder 用过了。 | ||
| 25 | + | ||
| 26 | +根因不是"LLM 没读提示词"这么简单——`src/work_graph_plan.py` 里 Coder 和 Validator 的建议节点 | ||
| 27 | +目录(`_CODER_NODE_CATALOG`/`_VALIDATOR_NODE_CATALOG`)**都**给"写一个可机械验证的测试脚本"这类 | ||
| 28 | +节点建议了同一个 `node_type: "test_script"`。两个不同 org_node 在描述"概念上相似的东西"时,天然 | ||
| 29 | +会往同一个规范化命名(`n_test_script`)收敛——即使 prompt 里明确列出了"这些 id 已经被用了", | ||
| 30 | +自然语言指令本身也无法保证绝对不再犯。这不是这一次运气不好,是这个架构下的一个系统性碰撞概率 | ||
| 31 | +问题:**只要两个 org_node 的建议目录用相同/相似的 `node_type` 描述概念上相似的节点,撞名概率就 | ||
| 32 | +不是小概率事件**。 | ||
| 33 | + | ||
| 34 | +`PlanningFailedError` 不在 `scheduler._ROUTABLE_EXCEPTIONS` 白名单里,所以这次撞名直接变成了 | ||
| 35 | +`blocked_pending_human`/`execution_error`,需要人工介入——跟 | ||
| 36 | +`docs/todo/boss-agent-autonomous-blocked-task-arbiter-requirements.md` FR-7 是同一类问题(系统 | ||
| 37 | +自己能分类的原因,不该都堆进笼统的兜底桶),但这条更进一步:**这类碰撞,理想情况下根本不该走到 | ||
| 38 | +需要人工介入这一步**,应该有更根本的预防或自动恢复机制。 | ||
| 39 | + | ||
| 40 | +## 2. 目标 | ||
| 41 | + | ||
| 42 | +让追加规划(`append_to_run_id`)在遇到 `node_id` 跨 org_node 撞名时,不需要人工介入就能正确处理 | ||
| 43 | +(预防撞名,或者撞了之后自动恢复),而不是直接硬失败变成 `blocked_pending_human`。 | ||
| 44 | + | ||
| 45 | +## 3. 非目标 | ||
| 46 | + | ||
| 47 | +1. **不裁定具体用哪种方案**(见 §7)——命名规范前缀(比如按 org_node 加前缀)、失败后带反馈重 | ||
| 48 | + 规划(跟 planner_reviewer 的重试模式类似)、自动重命名并改写内部引用,三种代价和可靠性都不同, | ||
| 49 | + 需要设计阶段权衡。 | ||
| 50 | +2. **不改变 `_CODER_NODE_CATALOG`/`_VALIDATOR_NODE_CATALOG` 这两份目录已有的 `node_type` 分类 | ||
| 51 | + 本身**——"Coder 和 Validator 都需要一个 test_script 类型的节点"这个业务事实是对的、不用改,需要 | ||
| 52 | + 改的是"两者都可能选出同一个具体 `node_id` 字符串"这件事。 | ||
| 53 | + | ||
| 54 | +## 4. 功能需求 | ||
| 55 | + | ||
| 56 | +| # | 需求描述 | 优先级 | | ||
| 57 | +|---|---|---| | ||
| 58 | +| FR-1 | `append_to_run_id` 模式下发生 `node_id` 撞名时,不能直接终结为需要人工介入的 `blocked_pending_human`——需要预防(撞名概率显著降低)或自动恢复(撞名后不需要人工也能继续)之一,具体方案见 §7。 | 必须 | | ||
| 59 | +| FR-2 | 如果最终方案里撞名之后仍然可能真正无法自动恢复(比如重试次数用尽),这种情况下打的原因也要专属、可区分,不能落入笼统的 `execution_error`(跟 Boss Agent 文档 FR-7 同一方向)。 | 必须 | | ||
| 60 | + | ||
| 61 | +## 5. 验收标准 | ||
| 62 | + | ||
| 63 | +构造一个真实场景:两个不同 org_node(比如 Coder 和 Validator)在同一个共享 run 里,各自的 | ||
| 64 | +建议目录都指向同一个 `node_type`(如 `test_script`),触发一次真实的 `claude -p` 自主规划——验证 | ||
| 65 | +最终不会因为 `node_id` 撞名而进入需要人工介入的状态。 | ||
| 66 | + | ||
| 67 | +## 6. 风险 | ||
| 68 | + | ||
| 69 | +如果选择"自动重命名"类方案,需要非常小心地同步改写这个新节点自己内部的 `depends_on`/ | ||
| 70 | +`on_failure` 引用(如果它引用了自己),改写不完整会引入比"直接拒绝"更隐蔽的错误(图结构损坏, | ||
| 71 | +但没有任何报错,节点执行时才会诡异地找不到依赖)——这条风险比"要不要做"更需要在设计阶段认真 | ||
| 72 | +对待。 | ||
| 73 | + | ||
| 74 | +## 7. 开放问题 | ||
| 75 | + | ||
| 76 | +- [ ] **具体用哪种方案**: | ||
| 77 | + - (a) 命名规范前缀——要求所有自规划节点的 `node_id` 强制带 org_node 前缀(如 | ||
| 78 | + `validator_test_script`),从命名规则上让跨 org_node 撞名在正常情况下不可能发生。最简单、 | ||
| 79 | + 风险最低,但需要确认这不会破坏已有的 `_normalize_graph_manifest`/`_extract_graph_manifest` | ||
| 80 | + 对 `node_id` 格式的既有假设。 | ||
| 81 | + - (b) 撞名后带反馈重规划——检测到撞名时不直接失败,而是把撞名信息作为反馈重新调用一次规划 | ||
| 82 | + (类似 `_review_planner_output_with_retries` 的重试模式),要求换一个不同的 id。需要设计 | ||
| 83 | + 重试上限。 | ||
| 84 | + - (c) 自动重命名——检测到撞名时自动加后缀改名并同步改写这次新规划里所有对该 id 的内部引用。 | ||
| 85 | + 见 §6 的风险,代价最高。 | ||
| 86 | +- [ ] 跟 `docs/todo/boss-agent-autonomous-blocked-task-arbiter-requirements.md`/ | ||
| 87 | + `docs/todo/agent-call-liveness-not-fixed-duration-timeout-requirements.md` 的实现顺序: | ||
| 88 | + 三份文档都在处理"什么情况不该变成需要人工介入的 blocked_pending_human",建议一起规划实现 | ||
| 89 | + 顺序,不要三个 Goal 各自独立设计出三套不一致的"是否需要人工介入"判断逻辑。 |
frontend/app.js
| @@ -939,7 +939,7 @@ function renderTaskBoard() { | @@ -939,7 +939,7 @@ function renderTaskBoard() { | ||
| 939 | </div> | 939 | </div> |
| 940 | <table class="data-table"> | 940 | <table class="data-table"> |
| 941 | <thead> | 941 | <thead> |
| 942 | - <tr><th>task_id</th><th>goal_text</th><th>status</th><th>assigned_org_node</th><th>loop_round</th><th>updated_at</th></tr> | 942 | + <tr><th>task_id</th><th>goal_text</th><th>status</th><th>assigned_org_node</th><th>loop_round</th><th>updated_at</th><th>执行顺序</th></tr> |
| 943 | </thead> | 943 | </thead> |
| 944 | <tbody id="tb_rows"></tbody> | 944 | <tbody id="tb_rows"></tbody> |
| 945 | </table> | 945 | </table> |
| @@ -984,7 +984,7 @@ function mountTaskBoard(queryParams) { | @@ -984,7 +984,7 @@ function mountTaskBoard(queryParams) { | ||
| 984 | const resp = await fetch("/api/tasks?" + params.toString()); | 984 | const resp = await fetch("/api/tasks?" + params.toString()); |
| 985 | const tasks = await resp.json(); | 985 | const tasks = await resp.json(); |
| 986 | if (tasks.length === 0) { | 986 | if (tasks.length === 0) { |
| 987 | - rowsEl.innerHTML = '<tr><td colspan="6">暂无任务</td></tr>'; | 987 | + rowsEl.innerHTML = '<tr><td colspan="7">暂无任务</td></tr>'; |
| 988 | return; | 988 | return; |
| 989 | } | 989 | } |
| 990 | rowsEl.innerHTML = tasks.map((t) => ( | 990 | rowsEl.innerHTML = tasks.map((t) => ( |
| @@ -999,6 +999,15 @@ function mountTaskBoard(queryParams) { | @@ -999,6 +999,15 @@ function mountTaskBoard(queryParams) { | ||
| 999 | + "<td>" + t.assigned_org_node + "</td>" | 999 | + "<td>" + t.assigned_org_node + "</td>" |
| 1000 | + "<td>" + t.loop_round + "</td>" | 1000 | + "<td>" + t.loop_round + "</td>" |
| 1001 | + "<td>" + escapeHtml(t.updated_at) + "</td>" | 1001 | + "<td>" + escapeHtml(t.updated_at) + "</td>" |
| 1002 | + // 2026-08-13 (real incident retrospective): manually-assignable | ||
| 1003 | + // execution-order hint -- serial steps are whole numbers (1, 2, 3), | ||
| 1004 | + // parallel siblings of one step are decimals sharing the same | ||
| 1005 | + // integer part (2.1, 2.2). Purely advisory (see task.execution_priority's | ||
| 1006 | + // own schema comment) -- editing this never blocks/reorders real | ||
| 1007 | + // execution, it's a planning aid rendered here for visibility. | ||
| 1008 | + + '<td><input type="number" step="0.1" class="tb-priority-input" ' | ||
| 1009 | + + 'data-task-id="' + t.task_id + '" value="' + (t.execution_priority == null ? "" : t.execution_priority) + '" ' | ||
| 1010 | + + 'placeholder="—" style="width:4.5em;" onclick="event.stopPropagation()"></td>' | ||
| 1002 | + "</tr>" | 1011 | + "</tr>" |
| 1003 | )).join(""); | 1012 | )).join(""); |
| 1004 | for (const row of rowsEl.querySelectorAll("tr[data-task-id]")) { | 1013 | for (const row of rowsEl.querySelectorAll("tr[data-task-id]")) { |
| @@ -1006,6 +1015,17 @@ function mountTaskBoard(queryParams) { | @@ -1006,6 +1015,17 @@ function mountTaskBoard(queryParams) { | ||
| 1006 | window.location.hash = "#/tasks/" + row.dataset.taskId; | 1015 | window.location.hash = "#/tasks/" + row.dataset.taskId; |
| 1007 | }); | 1016 | }); |
| 1008 | } | 1017 | } |
| 1018 | + for (const input of rowsEl.querySelectorAll(".tb-priority-input")) { | ||
| 1019 | + input.addEventListener("change", async () => { | ||
| 1020 | + const raw = input.value.trim(); | ||
| 1021 | + const execution_priority = raw === "" ? null : Number(raw); | ||
| 1022 | + await fetch("/api/tasks/" + encodeURIComponent(input.dataset.taskId), { | ||
| 1023 | + method: "PUT", | ||
| 1024 | + headers: { "Content-Type": "application/json" }, | ||
| 1025 | + body: JSON.stringify({ execution_priority }), | ||
| 1026 | + }); | ||
| 1027 | + }); | ||
| 1028 | + } | ||
| 1009 | } | 1029 | } |
| 1010 | 1030 | ||
| 1011 | keywordEl.addEventListener("input", refresh); | 1031 | keywordEl.addEventListener("input", refresh); |
src/backend_server.py
| @@ -459,16 +459,29 @@ class _Handler(BaseHTTPRequestHandler): | @@ -459,16 +459,29 @@ class _Handler(BaseHTTPRequestHandler): | ||
| 459 | self._send_json(400, error) | 459 | self._send_json(400, error) |
| 460 | return | 460 | return |
| 461 | goal_text = payload.get("goal_text") | 461 | goal_text = payload.get("goal_text") |
| 462 | - if not goal_text: | ||
| 463 | - self._send_json(400, {"error": "INVALID_PARAM", "detail": "goal_text is required"}) | 462 | + # 2026-08-13: execution_priority is an independent, optional field on |
| 463 | + # this same PUT -- unlike goal_text it's never gated to status= | ||
| 464 | + # 'queued' (see update_task_execution_priority's own docstring), so | ||
| 465 | + # it's handled as its own branch rather than folded into the | ||
| 466 | + # goal_text-only contract this endpoint originally had. | ||
| 467 | + has_priority = "execution_priority" in payload | ||
| 468 | + if not goal_text and not has_priority: | ||
| 469 | + self._send_json(400, {"error": "INVALID_PARAM", "detail": "goal_text or execution_priority is required"}) | ||
| 464 | return | 470 | return |
| 465 | try: | 471 | try: |
| 466 | with self.server.conn_lock: | 472 | with self.server.conn_lock: |
| 467 | - result = task_query.update_task_goal_text(self.server.conn, task_id, goal_text) | 473 | + result = None |
| 474 | + if goal_text: | ||
| 475 | + result = task_query.update_task_goal_text(self.server.conn, task_id, goal_text) | ||
| 476 | + if has_priority: | ||
| 477 | + result = task_query.update_task_execution_priority( | ||
| 478 | + self.server.conn, task_id, payload.get("execution_priority") | ||
| 479 | + ) | ||
| 468 | except task_query.TaskNotFoundError: | 480 | except task_query.TaskNotFoundError: |
| 469 | self._send_json(404, {"error": "NOT_FOUND"}) | 481 | self._send_json(404, {"error": "NOT_FOUND"}) |
| 470 | - except task_query.TaskNotEditableError as e: | ||
| 471 | - self._send_json(409, {"error": "TASK_NOT_EDITABLE", "detail": str(e)}) | 482 | + except (task_query.TaskNotEditableError, task_query.InvalidParamError) as e: |
| 483 | + status = 409 if isinstance(e, task_query.TaskNotEditableError) else 400 | ||
| 484 | + self._send_json(status, {"error": "TASK_NOT_EDITABLE" if status == 409 else "INVALID_PARAM", "detail": str(e)}) | ||
| 472 | else: | 485 | else: |
| 473 | self._send_json(200, result) | 486 | self._send_json(200, result) |
| 474 | 487 | ||
| @@ -1043,6 +1056,7 @@ def _ensure_schema(conn): | @@ -1043,6 +1056,7 @@ def _ensure_schema(conn): | ||
| 1043 | schema.migrate_relax_agent_profile_migrated_from_profile(conn) | 1056 | schema.migrate_relax_agent_profile_migrated_from_profile(conn) |
| 1044 | schema.migrate_drop_work_graph_run_org_node(conn) | 1057 | schema.migrate_drop_work_graph_run_org_node(conn) |
| 1045 | schema.migrate_add_concurrency_wait_event_type(conn) | 1058 | schema.migrate_add_concurrency_wait_event_type(conn) |
| 1059 | + schema.migrate_add_task_execution_priority(conn) | ||
| 1046 | 1060 | ||
| 1047 | 1061 | ||
| 1048 | class _Server(ThreadingHTTPServer): | 1062 | class _Server(ThreadingHTTPServer): |
src/schema.py
| @@ -116,6 +116,15 @@ CREATE TABLE task ( | @@ -116,6 +116,15 @@ CREATE TABLE task ( | ||
| 116 | blocked_reason TEXT, | 116 | blocked_reason TEXT, |
| 117 | -- H (2026-08-06): see skill.owner_org_node's comment above -- same relaxation. | 117 | -- H (2026-08-06): see skill.owner_org_node's comment above -- same relaxation. |
| 118 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), | 118 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), |
| 119 | + -- 2026-08-13 (real incident retrospective): a manually-assignable | ||
| 120 | + -- execution-order hint across a project's tasks -- NULLable (most | ||
| 121 | + -- tasks never get one), REAL not INTEGER so parallel siblings of one | ||
| 122 | + -- serial step can be expressed as decimals (e.g. step 2 splits into | ||
| 123 | + -- 2.1/2.2, both meant to run concurrently, before step 3). Purely | ||
| 124 | + -- advisory: nothing in the scheduler/dispatch loop reads this column | ||
| 125 | + -- to gate execution -- it's a human/Boss-Agent-facing planning aid | ||
| 126 | + -- shown on v_task_board, not an enforcement mechanism. | ||
| 127 | + execution_priority REAL, | ||
| 119 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, | 128 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, |
| 120 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | 129 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP |
| 121 | ); | 130 | ); |
| @@ -494,11 +503,12 @@ def migrate_relax_org_node_constraints(conn): | @@ -494,11 +503,12 @@ def migrate_relax_org_node_constraints(conn): | ||
| 494 | can't be toggled mid-transaction), rebuild, PRAGMA foreign_key_check to | 503 | can't be toggled mid-transaction), rebuild, PRAGMA foreign_key_check to |
| 495 | confirm nothing went dangling, PRAGMA foreign_keys=ON again. | 504 | confirm nothing went dangling, PRAGMA foreign_keys=ON again. |
| 496 | 505 | ||
| 497 | - Depends on migrate_add_work_graph_fork_columns() having already run | ||
| 498 | - (called here unconditionally, it's idempotent) -- work_graph_run's | ||
| 499 | - rebuild needs to know about D's parent_run_id/fork_of_node_exec_id | ||
| 500 | - columns whether or not this specific database has seen that migration | ||
| 501 | - yet. | 506 | + Depends on migrate_add_work_graph_fork_columns() and (2026-08-13) |
| 507 | + migrate_add_task_execution_priority() having already run (both called | ||
| 508 | + here unconditionally, both idempotent) -- work_graph_run's rebuild | ||
| 509 | + needs to know about D's parent_run_id/fork_of_node_exec_id columns, and | ||
| 510 | + task's rebuild needs to know about execution_priority, whether or not | ||
| 511 | + this specific database has seen either migration yet. | ||
| 502 | 512 | ||
| 503 | Idempotent: each table's rebuild is skipped if its stored CREATE TABLE | 513 | Idempotent: each table's rebuild is skipped if its stored CREATE TABLE |
| 504 | text no longer contains the old enum CHECK, so a fresh | 514 | text no longer contains the old enum CHECK, so a fresh |
| @@ -506,6 +516,7 @@ def migrate_relax_org_node_constraints(conn): | @@ -506,6 +516,7 @@ def migrate_relax_org_node_constraints(conn): | ||
| 506 | same as migrate_expand_artifact_types's own convention. | 516 | same as migrate_expand_artifact_types's own convention. |
| 507 | """ | 517 | """ |
| 508 | migrate_add_work_graph_fork_columns(conn) | 518 | migrate_add_work_graph_fork_columns(conn) |
| 519 | + migrate_add_task_execution_priority(conn) | ||
| 509 | conn.commit() | 520 | conn.commit() |
| 510 | 521 | ||
| 511 | conn.execute("PRAGMA foreign_keys = OFF") | 522 | conn.execute("PRAGMA foreign_keys = OFF") |
| @@ -548,6 +559,7 @@ def migrate_relax_org_node_constraints(conn): | @@ -548,6 +559,7 @@ def migrate_relax_org_node_constraints(conn): | ||
| 548 | loop_round INTEGER NOT NULL DEFAULT 0, | 559 | loop_round INTEGER NOT NULL DEFAULT 0, |
| 549 | blocked_reason TEXT, | 560 | blocked_reason TEXT, |
| 550 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), | 561 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), |
| 562 | + execution_priority REAL, | ||
| 551 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, | 563 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, |
| 552 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | 564 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP |
| 553 | ); | 565 | ); |
| @@ -736,10 +748,17 @@ def migrate_add_reviewing_planner_output_status(conn): | @@ -736,10 +748,17 @@ def migrate_add_reviewing_planner_output_status(conn): | ||
| 736 | migrate_relax_org_node_constraints() precedent (that function's own | 748 | migrate_relax_org_node_constraints() precedent (that function's own |
| 737 | docstring explains the same PRAGMA foreign_keys=OFF/rebuild/ | 749 | docstring explains the same PRAGMA foreign_keys=OFF/rebuild/ |
| 738 | foreign_key_check/ON dance in more depth) instead of the leaf-table | 750 | foreign_key_check/ON dance in more depth) instead of the leaf-table |
| 739 | - executescript() shortcut this originally (wrongly) copied.""" | 751 | + executescript() shortcut this originally (wrongly) copied. |
| 752 | + | ||
| 753 | + Depends on (2026-08-13) migrate_add_task_execution_priority() having | ||
| 754 | + already run (called here unconditionally, it's idempotent) -- same | ||
| 755 | + "rebuild needs to know about a later additive column" reasoning as | ||
| 756 | + migrate_relax_org_node_constraints()'s own dependency on | ||
| 757 | + migrate_add_work_graph_fork_columns().""" | ||
| 740 | row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='task'").fetchone() | 758 | row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='task'").fetchone() |
| 741 | if row is None or "'reviewing_planner_output'" in row[0]: | 759 | if row is None or "'reviewing_planner_output'" in row[0]: |
| 742 | return | 760 | return |
| 761 | + migrate_add_task_execution_priority(conn) | ||
| 743 | conn.execute("PRAGMA foreign_keys = OFF") | 762 | conn.execute("PRAGMA foreign_keys = OFF") |
| 744 | try: | 763 | try: |
| 745 | conn.executescript( | 764 | conn.executescript( |
| @@ -761,6 +780,7 @@ def migrate_add_reviewing_planner_output_status(conn): | @@ -761,6 +780,7 @@ def migrate_add_reviewing_planner_output_status(conn): | ||
| 761 | loop_round INTEGER NOT NULL DEFAULT 0, | 780 | loop_round INTEGER NOT NULL DEFAULT 0, |
| 762 | blocked_reason TEXT, | 781 | blocked_reason TEXT, |
| 763 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), | 782 | assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), |
| 783 | + execution_priority REAL, | ||
| 764 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, | 784 | created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, |
| 765 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP | 785 | updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP |
| 766 | ); | 786 | ); |
| @@ -860,6 +880,19 @@ def migrate_add_concurrency_wait_event_type(conn): | @@ -860,6 +880,19 @@ def migrate_add_concurrency_wait_event_type(conn): | ||
| 860 | conn.commit() | 880 | conn.commit() |
| 861 | 881 | ||
| 862 | 882 | ||
| 883 | +def migrate_add_task_execution_priority(conn): | ||
| 884 | + """2026-08-13 (real incident retrospective): adds task.execution_priority | ||
| 885 | + on an ALREADY-POPULATED database. Plain `ALTER TABLE ... ADD COLUMN` (no | ||
| 886 | + CHECK constraint involved, no create-copy-drop-rename dance needed) -- | ||
| 887 | + same pattern as migrate_add_work_graph_fork_columns. Idempotent and safe | ||
| 888 | + to call unconditionally, including against a freshly create_schema()'d | ||
| 889 | + database (which already has the column).""" | ||
| 890 | + cols = {row[1] for row in conn.execute("PRAGMA table_info(task)").fetchall()} | ||
| 891 | + if "execution_priority" not in cols: | ||
| 892 | + conn.execute("ALTER TABLE task ADD COLUMN execution_priority REAL") | ||
| 893 | + conn.commit() | ||
| 894 | + | ||
| 895 | + | ||
| 863 | def migrate_drop_work_graph_run_org_node(conn): | 896 | def migrate_drop_work_graph_run_org_node(conn): |
| 864 | """Org Graph unification Phase 2c §2.1/§2.2 (2026-08-12, user-confirmed | 897 | """Org Graph unification Phase 2c §2.1/§2.2 (2026-08-12, user-confirmed |
| 865 | 2026-08-08): drops work_graph_run.org_node on an ALREADY-POPULATED | 898 | 2026-08-08): drops work_graph_run.org_node on an ALREADY-POPULATED |
src/task_query.py
| @@ -30,7 +30,7 @@ def list_tasks(conn, project_id, status=None, org_node=None, keyword=None, sort= | @@ -30,7 +30,7 @@ def list_tasks(conn, project_id, status=None, org_node=None, keyword=None, sort= | ||
| 30 | reaches the SQL text itself, only a pre-approved column name.""" | 30 | reaches the SQL text itself, only a pre-approved column name.""" |
| 31 | query = ( | 31 | query = ( |
| 32 | "SELECT task_id, goal_text, task_type, status, assigned_org_node, loop_round, " | 32 | "SELECT task_id, goal_text, task_type, status, assigned_org_node, loop_round, " |
| 33 | - "created_at, updated_at FROM task WHERE project_id = ?" | 33 | + "created_at, updated_at, execution_priority FROM task WHERE project_id = ?" |
| 34 | ) | 34 | ) |
| 35 | params = [project_id] | 35 | params = [project_id] |
| 36 | if status: | 36 | if status: |
| @@ -54,6 +54,7 @@ def list_tasks(conn, project_id, status=None, org_node=None, keyword=None, sort= | @@ -54,6 +54,7 @@ def list_tasks(conn, project_id, status=None, org_node=None, keyword=None, sort= | ||
| 54 | { | 54 | { |
| 55 | "task_id": r[0], "goal_text": r[1], "task_type": r[2], "status": r[3], | 55 | "task_id": r[0], "goal_text": r[1], "task_type": r[2], "status": r[3], |
| 56 | "assigned_org_node": r[4], "loop_round": r[5], "created_at": r[6], "updated_at": r[7], | 56 | "assigned_org_node": r[4], "loop_round": r[5], "created_at": r[6], "updated_at": r[7], |
| 57 | + "execution_priority": r[8], | ||
| 57 | } | 58 | } |
| 58 | for r in rows | 59 | for r in rows |
| 59 | ] | 60 | ] |
| @@ -88,6 +89,26 @@ def update_task_goal_text(conn, task_id, goal_text): | @@ -88,6 +89,26 @@ def update_task_goal_text(conn, task_id, goal_text): | ||
| 88 | return get_task(conn, task_id) | 89 | return get_task(conn, task_id) |
| 89 | 90 | ||
| 90 | 91 | ||
| 92 | +def update_task_execution_priority(conn, task_id, execution_priority): | ||
| 93 | + """2026-08-13 (real incident retrospective): unlike goal_text, this is | ||
| 94 | + NOT gated to status='queued' -- execution_priority is a scheduling/ | ||
| 95 | + display hint (v_task_board's new column), not a planning input that | ||
| 96 | + invalidates work already done, so it should stay settable regardless of | ||
| 97 | + a task's current status (including a task that's already blocked or | ||
| 98 | + mid-flight). None clears it back to unset.""" | ||
| 99 | + row = conn.execute("SELECT status FROM task WHERE task_id = ?", (task_id,)).fetchone() | ||
| 100 | + if row is None: | ||
| 101 | + raise TaskNotFoundError(task_id) | ||
| 102 | + if execution_priority is not None and not isinstance(execution_priority, (int, float)): | ||
| 103 | + raise InvalidParamError("execution_priority must be a number or null, got {!r}".format(execution_priority)) | ||
| 104 | + conn.execute( | ||
| 105 | + "UPDATE task SET execution_priority = ?, updated_at = CURRENT_TIMESTAMP WHERE task_id = ?", | ||
| 106 | + (execution_priority, task_id), | ||
| 107 | + ) | ||
| 108 | + conn.commit() | ||
| 109 | + return get_task(conn, task_id) | ||
| 110 | + | ||
| 111 | + | ||
| 91 | def delete_task(conn, task_id): | 112 | def delete_task(conn, task_id): |
| 92 | row = conn.execute("SELECT status FROM task WHERE task_id = ?", (task_id,)).fetchone() | 113 | row = conn.execute("SELECT status FROM task WHERE task_id = ?", (task_id,)).fetchone() |
| 93 | if row is None: | 114 | if row is None: |
| @@ -115,7 +136,8 @@ def delete_task(conn, task_id): | @@ -115,7 +136,8 @@ def delete_task(conn, task_id): | ||
| 115 | def get_task(conn, task_id): | 136 | def get_task(conn, task_id): |
| 116 | row = conn.execute( | 137 | row = conn.execute( |
| 117 | "SELECT task_id, project_id, goal_text, task_type, origin_ref, created_via, status, route, " | 138 | "SELECT task_id, project_id, goal_text, task_type, origin_ref, created_via, status, route, " |
| 118 | - "loop_round, blocked_reason, assigned_org_node, created_at, updated_at FROM task WHERE task_id = ?", | 139 | + "loop_round, blocked_reason, assigned_org_node, created_at, updated_at, execution_priority " |
| 140 | + "FROM task WHERE task_id = ?", | ||
| 119 | (task_id,), | 141 | (task_id,), |
| 120 | ).fetchone() | 142 | ).fetchone() |
| 121 | if row is None: | 143 | if row is None: |
| @@ -125,7 +147,7 @@ def get_task(conn, task_id): | @@ -125,7 +147,7 @@ def get_task(conn, task_id): | ||
| 125 | "task_id": row[0], "project_id": row[1], "goal_text": row[2], "task_type": row[3], | 147 | "task_id": row[0], "project_id": row[1], "goal_text": row[2], "task_type": row[3], |
| 126 | "origin_ref": row[4], "created_via": row[5], "status": row[6], "route": row[7], | 148 | "origin_ref": row[4], "created_via": row[5], "status": row[6], "route": row[7], |
| 127 | "loop_round": row[8], "blocked_reason": row[9], "assigned_org_node": row[10], | 149 | "loop_round": row[8], "blocked_reason": row[9], "assigned_org_node": row[10], |
| 128 | - "created_at": row[11], "updated_at": row[12], | 150 | + "created_at": row[11], "updated_at": row[12], "execution_priority": row[13], |
| 129 | } | 151 | } |
| 130 | 152 | ||
| 131 | artifacts = conn.execute( | 153 | artifacts = conn.execute( |
src/test_backend_server.py
| @@ -507,6 +507,21 @@ class TaskCrudRouteTest(unittest.TestCase): | @@ -507,6 +507,21 @@ class TaskCrudRouteTest(unittest.TestCase): | ||
| 507 | self.assertEqual(status, 409) | 507 | self.assertEqual(status, 409) |
| 508 | self.assertEqual(body["error"], "TASK_NOT_EDITABLE") | 508 | self.assertEqual(body["error"], "TASK_NOT_EDITABLE") |
| 509 | 509 | ||
| 510 | + def test_put_execution_priority_works_regardless_of_status(self): | ||
| 511 | + # 2026-08-13 (real incident retrospective): unlike goal_text, this | ||
| 512 | + # must succeed even on a non-queued task. | ||
| 513 | + self.server.conn.execute("UPDATE task SET status = 'coding' WHERE task_id = 'task-1'") | ||
| 514 | + self.server.conn.commit() | ||
| 515 | + status, body = self._put("/api/tasks/task-1", {"execution_priority": 2.1}) | ||
| 516 | + self.assertEqual(status, 200) | ||
| 517 | + self.assertEqual(body["execution_priority"], 2.1) | ||
| 518 | + | ||
| 519 | + def test_get_list_includes_execution_priority(self): | ||
| 520 | + self._put("/api/tasks/task-1", {"execution_priority": 3}) | ||
| 521 | + status, body = self._get("/api/tasks?project_id=proj-1") | ||
| 522 | + self.assertEqual(status, 200) | ||
| 523 | + self.assertEqual(body[0]["execution_priority"], 3) | ||
| 524 | + | ||
| 510 | def test_delete_removes_queued_task(self): | 525 | def test_delete_removes_queued_task(self): |
| 511 | status, body = self._delete("/api/tasks/task-1") | 526 | status, body = self._delete("/api/tasks/task-1") |
| 512 | self.assertEqual(status, 200) | 527 | self.assertEqual(status, 200) |
src/test_schema.py
| @@ -568,6 +568,58 @@ class MigrateAddWorkGraphForkColumnsTest(unittest.TestCase): | @@ -568,6 +568,58 @@ class MigrateAddWorkGraphForkColumnsTest(unittest.TestCase): | ||
| 568 | fresh.close() | 568 | fresh.close() |
| 569 | 569 | ||
| 570 | 570 | ||
| 571 | +class MigrateAddTaskExecutionPriorityTest(unittest.TestCase): | ||
| 572 | + """2026-08-13 (real incident retrospective): real upgrade path for an | ||
| 573 | + already-populated database whose task table predates | ||
| 574 | + execution_priority.""" | ||
| 575 | + | ||
| 576 | + def setUp(self): | ||
| 577 | + self.conn = schema.connect(":memory:") | ||
| 578 | + old_sql = re.sub( | ||
| 579 | + r",\n( --[^\n]*\n)* execution_priority REAL,\n", | ||
| 580 | + ",\n", | ||
| 581 | + schema.SCHEMA_SQL, | ||
| 582 | + ) | ||
| 583 | + self.assertNotIn("execution_priority", old_sql) # sanity: the substitution actually matched | ||
| 584 | + self.conn.executescript(old_sql) | ||
| 585 | + self.conn.commit() | ||
| 586 | + self.conn.execute( | ||
| 587 | + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " | ||
| 588 | + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" | ||
| 589 | + ) | ||
| 590 | + self.conn.execute( | ||
| 591 | + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " | ||
| 592 | + "VALUES ('task-1', 'proj-1', 'g', 'goal', 'queued', 'planner')" | ||
| 593 | + ) | ||
| 594 | + self.conn.commit() | ||
| 595 | + | ||
| 596 | + def tearDown(self): | ||
| 597 | + self.conn.close() | ||
| 598 | + | ||
| 599 | + def test_migration_preserves_existing_rows_and_adds_nullable_column(self): | ||
| 600 | + schema.migrate_add_task_execution_priority(self.conn) | ||
| 601 | + row = self.conn.execute( | ||
| 602 | + "SELECT status, execution_priority FROM task WHERE task_id = 'task-1'" | ||
| 603 | + ).fetchone() | ||
| 604 | + self.assertEqual(row, ("queued", None)) | ||
| 605 | + | ||
| 606 | + def test_idempotent_on_a_database_already_migrated(self): | ||
| 607 | + schema.migrate_add_task_execution_priority(self.conn) | ||
| 608 | + schema.migrate_add_task_execution_priority(self.conn) # must not raise | ||
| 609 | + count = self.conn.execute("SELECT COUNT(*) FROM task").fetchone()[0] | ||
| 610 | + self.assertEqual(count, 1) | ||
| 611 | + | ||
| 612 | + def test_noop_on_a_freshly_created_database(self): | ||
| 613 | + fresh = schema.connect(":memory:") | ||
| 614 | + try: | ||
| 615 | + schema.create_schema(fresh) | ||
| 616 | + schema.migrate_add_task_execution_priority(fresh) # must not raise | ||
| 617 | + cols = {row[1] for row in fresh.execute("PRAGMA table_info(task)").fetchall()} | ||
| 618 | + self.assertIn("execution_priority", cols) | ||
| 619 | + finally: | ||
| 620 | + fresh.close() | ||
| 621 | + | ||
| 622 | + | ||
| 571 | _OLD_WORK_GRAPH_RUN_BLOCK = """-- 1.3 work_graph_run | 623 | _OLD_WORK_GRAPH_RUN_BLOCK = """-- 1.3 work_graph_run |
| 572 | CREATE TABLE work_graph_run ( | 624 | CREATE TABLE work_graph_run ( |
| 573 | run_id TEXT PRIMARY KEY, | 625 | run_id TEXT PRIMARY KEY, |
src/test_task_query.py
| @@ -340,6 +340,37 @@ class UpdateTaskGoalTextTest(TaskQueryTestBase): | @@ -340,6 +340,37 @@ class UpdateTaskGoalTextTest(TaskQueryTestBase): | ||
| 340 | task_query.update_task_goal_text(self.conn, "task-1", "new text") | 340 | task_query.update_task_goal_text(self.conn, "task-1", "new text") |
| 341 | 341 | ||
| 342 | 342 | ||
| 343 | +class UpdateTaskExecutionPriorityTest(TaskQueryTestBase): | ||
| 344 | + """2026-08-13 (real incident retrospective): unlike goal_text, this is | ||
| 345 | + NOT gated to status='queued' -- it's a scheduling/display hint, not a | ||
| 346 | + planning input.""" | ||
| 347 | + | ||
| 348 | + def test_in_progress_task_can_still_be_edited(self): | ||
| 349 | + self._insert_task("task-1", status="coding") | ||
| 350 | + result = task_query.update_task_execution_priority(self.conn, "task-1", 2.1) | ||
| 351 | + self.assertEqual(result["execution_priority"], 2.1) | ||
| 352 | + | ||
| 353 | + def test_serial_integer_value(self): | ||
| 354 | + self._insert_task("task-1", status="queued") | ||
| 355 | + result = task_query.update_task_execution_priority(self.conn, "task-1", 1) | ||
| 356 | + self.assertEqual(result["execution_priority"], 1) | ||
| 357 | + | ||
| 358 | + def test_none_clears_it(self): | ||
| 359 | + self._insert_task("task-1", status="queued") | ||
| 360 | + task_query.update_task_execution_priority(self.conn, "task-1", 3) | ||
| 361 | + result = task_query.update_task_execution_priority(self.conn, "task-1", None) | ||
| 362 | + self.assertIsNone(result["execution_priority"]) | ||
| 363 | + | ||
| 364 | + def test_nonexistent_task_raises_not_found(self): | ||
| 365 | + with self.assertRaises(task_query.TaskNotFoundError): | ||
| 366 | + task_query.update_task_execution_priority(self.conn, "no-such-task", 1) | ||
| 367 | + | ||
| 368 | + def test_non_numeric_value_raises_invalid_param(self): | ||
| 369 | + self._insert_task("task-1", status="queued") | ||
| 370 | + with self.assertRaises(task_query.InvalidParamError): | ||
| 371 | + task_query.update_task_execution_priority(self.conn, "task-1", "not a number") | ||
| 372 | + | ||
| 373 | + | ||
| 343 | class DeleteTaskTest(TaskQueryTestBase): | 374 | class DeleteTaskTest(TaskQueryTestBase): |
| 344 | def test_queued_task_can_be_deleted(self): | 375 | def test_queued_task_can_be_deleted(self): |
| 345 | self._insert_task("task-1", status="queued") | 376 | self._insert_task("task-1", status="queued") |