diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 2ccb0e0d1e9..4940b7fa5a1 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -107,15 +107,35 @@ def _honcho_is_configured_for_doctor() -> bool: return False +def _is_kanban_worker_env_gate(item: dict) -> bool: + """Return True when Kanban is unavailable only because this is not a worker process.""" + if item.get("name") != "kanban": + return False + if os.environ.get("HERMES_KANBAN_TASK"): + return False + + tools = item.get("tools") or [] + return bool(tools) and all(str(tool).startswith("kanban_") for tool in tools) + + +def _doctor_tool_availability_detail(toolset: str) -> str: + """Optional explanatory suffix for toolsets whose doctor status needs context.""" + if toolset == "kanban" and not os.environ.get("HERMES_KANBAN_TASK"): + return "(runtime-gated; loaded only for dispatcher-spawned workers)" + return "" + + def _apply_doctor_tool_availability_overrides(available: list[str], unavailable: list[dict]) -> tuple[list[str], list[dict]]: """Adjust runtime-gated tool availability for doctor diagnostics.""" - if not _honcho_is_configured_for_doctor(): - return available, unavailable - updated_available = list(available) updated_unavailable = [] for item in unavailable: - if item.get("name") == "honcho": + name = item.get("name") + if _is_kanban_worker_env_gate(item): + if "kanban" not in updated_available: + updated_available.append("kanban") + continue + if name == "honcho" and _honcho_is_configured_for_doctor(): if "honcho" not in updated_available: updated_available.append("honcho") continue @@ -1278,7 +1298,7 @@ def run_doctor(args): for tid in available: info = TOOLSET_REQUIREMENTS.get(tid, {}) - check_ok(info.get("name", tid)) + check_ok(info.get("name", tid), _doctor_tool_availability_detail(tid)) for item in unavailable: env_vars = item.get("missing_vars") or item.get("env_vars") or [] diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 87f3b7f9d1e..d8bc47a7d7b 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -1071,10 +1071,16 @@ def _cmd_show(args: argparse.Namespace) -> int: parents = kb.parent_ids(conn, args.task_id) children = kb.child_ids(conn, args.task_id) runs = kb.list_runs(conn, args.task_id) + # Workers hand off via ``task_runs.summary`` (kanban-worker skill); + # ``tasks.result`` is left NULL unless the caller explicitly passed + # ``result=``. Surfacing the latest summary here keeps ``show`` from + # looking like a no-op when the worker actually did real work. + latest_summary = kb.latest_summary(conn, args.task_id) if getattr(args, "json", False): payload = { "task": _task_to_dict(task), + "latest_summary": latest_summary, "parents": parents, "children": children, "comments": [ @@ -1161,6 +1167,13 @@ def _cmd_show(args: argparse.Namespace) -> int: print() print("Result:") print(task.result) + elif latest_summary: + # Worker handoff lives on the latest run, not on tasks.result. + # Surface it at top-level so a glance at ``hermes kanban show `` + # tells you what the worker did even if tasks.result is empty. + print() + print("Latest summary:") + print(latest_summary) if comments: print() print(f"Comments ({len(comments)}):") diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 97f24c435b9..8440113c25e 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -1978,14 +1978,23 @@ def _verify_created_cards( ) -> tuple[list[str], list[str]]: """Partition ``claimed_ids`` into (verified, phantom). - A card is "verified" iff a row exists in ``tasks`` with the given id - AND ``created_by`` matches the completing task's ``assignee`` (or - the completing task itself — workers that create children of their - own task also qualify). - - ``phantom`` returns ids that either don't exist at all or exist but - were not created by the completing worker. The caller decides what - to do with each bucket; this helper never mutates. + A card is "verified" iff a row exists in ``tasks`` AND at least one + of the following holds: + + * ``created_by`` matches the completing task's ``assignee`` profile + (the common case: worker A spawns a card via ``kanban_create``, + which stamps ``created_by=A``). + * ``created_by`` matches the completing task's id (edge case where + a worker passed its own task id as the ``created_by`` value). + * The card is linked as a ``task_links.child`` of the completing + task — i.e. the worker explicitly called ``kanban_create`` with + ``parents=[]``. This accepts cards created through + the dashboard/CLI by a different principal but then attached to + the completing task by the worker. + + ``phantom`` returns ids that either don't exist at all, or exist + but don't satisfy any of the three trust conditions. The caller + decides what to do with each bucket; this helper never mutates. """ claimed = [str(x).strip() for x in (claimed_ids or []) if str(x).strip()] if not claimed: @@ -2014,6 +2023,10 @@ def _verify_created_cards( ).fetchall() found = {r["id"]: r["created_by"] for r in rows} + # Pull the set of cards linked as children of the completing task. + # Cheap: one query, indexed on parent_id. + linked_children: set[str] = set(child_ids(conn, completing_task_id)) + verified: list[str] = [] phantom: list[str] = [] for cid in ordered: @@ -2021,13 +2034,13 @@ def _verify_created_cards( if created_by is None: phantom.append(cid) continue - # Accept if created_by matches the completing task's assignee - # profile, OR the task itself (workers whose created_by happens - # to match their task id are unusual but harmless to accept). + # Accept if any of the three trust conditions holds. if completing_assignee and created_by == completing_assignee: verified.append(cid) elif created_by == completing_task_id: verified.append(cid) + elif cid in linked_children: + verified.append(cid) else: phantom.append(cid) return verified, phantom @@ -2699,16 +2712,23 @@ def enforce_max_runtime( host_prefix = f"{_claimer_id().split(':', 1)[0]}:" rows = conn.execute( - "SELECT id, worker_pid, started_at, max_runtime_seconds, claim_lock " - "FROM tasks " - "WHERE status = 'running' AND max_runtime_seconds IS NOT NULL " - " AND started_at IS NOT NULL AND worker_pid IS NOT NULL" + "SELECT t.id, t.worker_pid, " + " COALESCE(r.started_at, t.started_at) AS active_started_at, " + " t.max_runtime_seconds, t.claim_lock " + "FROM tasks t " + "LEFT JOIN task_runs r ON r.id = t.current_run_id " + "WHERE t.status = 'running' AND t.max_runtime_seconds IS NOT NULL " + " AND COALESCE(r.started_at, t.started_at) IS NOT NULL " + " AND t.worker_pid IS NOT NULL" ).fetchall() for row in rows: lock = row["claim_lock"] or "" if not lock.startswith(host_prefix): continue - elapsed = now - int(row["started_at"]) + # Runtime is per attempt, not lifetime-of-task. ``tasks.started_at`` + # intentionally records the first time a task ever started, so retries + # must be measured from the active task_runs row when present. + elapsed = now - int(row["active_started_at"]) if elapsed < int(row["max_runtime_seconds"]): continue @@ -3993,3 +4013,61 @@ def latest_run(conn: sqlite3.Connection, task_id: str) -> Optional[Run]: (task_id,), ).fetchone() return Run.from_row(row) if row else None + + +def latest_summary(conn: sqlite3.Connection, task_id: str) -> Optional[str]: + """Return the latest non-null ``task_runs.summary`` for ``task_id``. + + The kanban-worker skill writes its handoff to ``task_runs.summary`` + via ``complete_task(summary=...)``; ``tasks.result`` is left empty + unless the caller passes ``result=`` explicitly. Dashboards and CLI + "show" views need this value to surface what a worker actually did + — without it, ``tasks.result`` is NULL and the task looks like a + no-op even when the run completed. + + Picks the most recent run by ``ended_at`` (falling back to ``id`` + for ties or unfinished rows). Returns None if no run has a summary. + """ + row = conn.execute( + "SELECT summary FROM task_runs " + "WHERE task_id = ? AND summary IS NOT NULL AND summary != '' " + "ORDER BY COALESCE(ended_at, started_at) DESC, id DESC LIMIT 1", + (task_id,), + ).fetchone() + return row["summary"] if row else None + + +def latest_summaries( + conn: sqlite3.Connection, task_ids: Iterable[str] +) -> dict[str, str]: + """Batch-fetch latest non-null summaries for a list of task ids. + + Used by the dashboard board endpoint to attach ``latest_summary`` to + every card in a single SQL query, avoiding the N+1 pattern of + calling :func:`latest_summary` per task. Returns a dict mapping + ``task_id`` → summary string, omitting tasks with no summary. + + Approach: a window function picks the newest non-null-summary row + per ``task_id``; works against SQLite ≥ 3.25 (default on every + supported platform). + """ + ids = list(task_ids) + if not ids: + return {} + placeholders = ",".join("?" for _ in ids) + rows = conn.execute( + f""" + SELECT task_id, summary FROM ( + SELECT task_id, summary, + ROW_NUMBER() OVER ( + PARTITION BY task_id + ORDER BY COALESCE(ended_at, started_at) DESC, id DESC + ) AS rn + FROM task_runs + WHERE task_id IN ({placeholders}) + AND summary IS NOT NULL AND summary != '' + ) WHERE rn = 1 + """, + ids, + ).fetchall() + return {r["task_id"]: r["summary"] for r in rows} diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index 02935b73ebc..b4d85432d83 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -2416,11 +2416,10 @@ ), ), h("div", { className: "hermes-kanban-deps-row" }, - h(Select, { + h(Select, Object.assign({ value: newParent, - onChange: function (e) { setNewParent(e.target.value); }, className: "h-7 text-xs flex-1", - }, + }, selectChangeHandler(setNewParent)), h(SelectOption, { value: "" }, "— add parent —"), candidatesFor(parentExclude).map(function (t) { return h(SelectOption, { key: t.id, value: t.id }, @@ -2455,11 +2454,10 @@ ), ), h("div", { className: "hermes-kanban-deps-row" }, - h(Select, { + h(Select, Object.assign({ value: newChild, - onChange: function (e) { setNewChild(e.target.value); }, className: "h-7 text-xs flex-1", - }, + }, selectChangeHandler(setNewChild)), h(SelectOption, { value: "" }, "— add child —"), candidatesFor(childExclude).map(function (t) { return h(SelectOption, { key: t.id, value: t.id }, diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index 2b5bcd0dad2..3176737a8ca 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -124,11 +124,23 @@ def _conn(board: Optional[str] = None): ] -def _task_dict(task: kanban_db.Task) -> dict[str, Any]: +_CARD_SUMMARY_PREVIEW_CHARS = 200 + + +def _task_dict( + task: kanban_db.Task, + *, + latest_summary: Optional[str] = None, +) -> dict[str, Any]: d = asdict(task) # Add derived age metrics so the UI can colour stale cards without # computing deltas client-side. d["age"] = kanban_db.task_age(task) + # Surface the latest non-null run summary so dashboards don't show + # blank cards/drawers for tasks where the worker handed off via + # ``task_runs.summary`` (the kanban-worker pattern) instead of + # ``tasks.result``. ``None`` when no run has produced a summary yet. + d["latest_summary"] = latest_summary # Keep body short on list endpoints; full body comes from /tasks/:id. return d @@ -381,8 +393,18 @@ def get_board( if include_archived: columns["archived"] = [] + # Batch-fetch the latest non-null run summary per task in one + # window-function query (avoids N+1 ``latest_summary`` calls + # for boards with hundreds of tasks). Truncated to a card-size + # preview here — the full text is available via /tasks/:id. + summary_map = kanban_db.latest_summaries(conn, [t.id for t in tasks]) + for t in tasks: - d = _task_dict(t) + full = summary_map.get(t.id) + preview = ( + full[:_CARD_SUMMARY_PREVIEW_CHARS] if full else None + ) + d = _task_dict(t, latest_summary=preview) d["link_counts"] = link_counts.get(t.id, {"parents": 0, "children": 0}) d["comment_count"] = comment_counts.get(t.id, 0) d["progress"] = progress.get(t.id) # None when the task has no children @@ -440,7 +462,11 @@ def get_task(task_id: str, board: Optional[str] = Query(None)): task = kanban_db.get_task(conn, task_id) if task is None: raise HTTPException(status_code=404, detail=f"task {task_id} not found") - task_d = _task_dict(task) + # Drawer/detail view returns the FULL summary (no truncation) so + # operators can read the complete worker handoff without making + # a second round-trip. Cards on /board carry a 200-char preview. + full_summary = kanban_db.latest_summary(conn, task_id) + task_d = _task_dict(task, latest_summary=full_summary) # Attach diagnostics so the drawer's Diagnostics section can # render recovery actions without a second round-trip. diags = _compute_task_diagnostics(conn, task_ids=[task_id]) @@ -662,6 +688,22 @@ def _set_status_direct( ).fetchone() if prev is None: return False + + # Guard: don't allow promoting to 'ready' unless all parents are done. + # Prevents the dispatcher from spawning a child whose upstream work + # hasn't completed (e.g. T4 dispatched while T3 is still blocked). + if new_status == "ready": + parent_statuses = conn.execute( + "SELECT t.status FROM tasks t " + "JOIN task_links l ON l.parent_id = t.id " + "WHERE l.child_id = ?", + (task_id,), + ).fetchall() + if parent_statuses and not all( + p["status"] == "done" for p in parent_statuses + ): + return False + was_running = prev["status"] == "running" cur = conn.execute( diff --git a/scripts/release.py b/scripts/release.py index aa9d6ead890..39ac44401a3 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -129,6 +129,8 @@ "momowind@gmail.com": "momowind", "clockwork-codex@users.noreply.github.com": "misery-hl", "207811921+misery-hl@users.noreply.github.com": "misery-hl", + "suncokret@protonmail.com": "suncokret12", + "mio.imoto.ai@gmail.com": "mioimotoai-lgtm", "aamirjawaid@microsoft.com": "heyitsaamir", "johnnncenaaa77@gmail.com": "johnncenae", "thomasjhon6666@gmail.com": "ThomassJonax", diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index 0f48606141a..374ef2dea4a 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -126,6 +126,47 @@ def test_leaves_honcho_unavailable_when_not_configured(self, monkeypatch): assert available == [] assert unavailable == [honcho_entry] + def test_marks_kanban_available_only_when_missing_worker_env_gate(self, monkeypatch): + monkeypatch.setattr(doctor, "_honcho_is_configured_for_doctor", lambda: False) + monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) + + available, unavailable = doctor._apply_doctor_tool_availability_overrides( + [], + [{"name": "kanban", "env_vars": [], "tools": ["kanban_show"]}], + ) + + assert available == ["kanban"] + assert unavailable == [] + + def test_leaves_kanban_unavailable_when_worker_env_is_set(self, monkeypatch): + monkeypatch.setenv("HERMES_KANBAN_TASK", "probe") + kanban_entry = {"name": "kanban", "env_vars": [], "tools": ["kanban_show"]} + + available, unavailable = doctor._apply_doctor_tool_availability_overrides( + [], + [kanban_entry], + ) + + assert available == [] + assert unavailable == [kanban_entry] + + def test_leaves_non_worker_kanban_failure_unavailable(self, monkeypatch): + monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) + kanban_entry = {"name": "kanban", "env_vars": [], "tools": ["kanban_show", "not_a_kanban_tool"]} + + available, unavailable = doctor._apply_doctor_tool_availability_overrides( + [], + [kanban_entry], + ) + + assert available == [] + assert unavailable == [kanban_entry] + + def test_kanban_doctor_detail_explains_worker_gate(self, monkeypatch): + monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) + + assert doctor._doctor_tool_availability_detail("kanban") == "(runtime-gated; loaded only for dispatcher-spawned workers)" + class TestHonchoDoctorConfigDetection: def test_reports_configured_when_enabled_with_api_key(self, monkeypatch): diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index 219aa2546d2..95dfdae82dc 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -682,14 +682,21 @@ def _signal_fn(pid, sig): conn, title="long job", assignee="worker", max_runtime_seconds=1, # one second cap ) - # Spawn by hand: claim + set pid + set started_at to the past. + # Spawn by hand: claim + set pid + set active run start to the past. kb.claim_task(conn, tid) kb._set_worker_pid(conn, tid, os.getpid()) # any live pid works - # Backdate started_at so elapsed > limit. + # Backdate both the task-level first-start timestamp and the active + # run timestamp so elapsed > limit under the per-run runtime model. + old_started = int(time.time()) - 30 with kb.write_txn(conn): conn.execute( "UPDATE tasks SET started_at = ? WHERE id = ?", - (int(time.time()) - 30, tid), + (old_started, tid), + ) + conn.execute( + "UPDATE task_runs SET started_at = ? " + "WHERE id = (SELECT current_run_id FROM tasks WHERE id = ?)", + (old_started, tid), ) timed_out = kb.enforce_max_runtime(conn, signal_fn=_signal_fn) @@ -769,10 +776,16 @@ def _signal(pid, sig): ) kb.claim_task(conn, tid) kb._set_worker_pid(conn, tid, os.getpid()) + old_started = int(time.time()) - 30 with kb.write_txn(conn): conn.execute( "UPDATE tasks SET started_at = ? WHERE id = ?", - (int(time.time()) - 30, tid), + (old_started, tid), + ) + conn.execute( + "UPDATE task_runs SET started_at = ? " + "WHERE id = (SELECT current_run_id FROM tasks WHERE id = ?)", + (old_started, tid), ) # Use enforce_max_runtime directly with our signal stub — dispatch_once # uses the default os.kill, but integration-wise calling @@ -2978,6 +2991,46 @@ def test_complete_with_cross_worker_card_is_rejected(kanban_home): conn.close() +def test_complete_accepts_cross_worker_card_when_linked_as_child(kanban_home): + """A card created by a different principal but explicitly linked as + a child of the completing task is accepted — the worker took + ownership via ``kanban_create(parents=[current_task])`` or an + explicit ``link_tasks`` call, which proves the relationship even + when ``created_by`` doesn't match. + + (Relaxation salvaged from #20022 @LeonSGP43 — stricter version + would incorrectly reject legitimate orchestrator flows where a + specifier creates a card, then a worker picks it up and links it + to its own parent task.) + """ + conn = kb.connect() + try: + parent = kb.create_task(conn, title="parent", assignee="alice") + # Card created by a DIFFERENT principal (not alice, not parent). + other = kb.create_task( + conn, title="other", assignee="x", created_by="bob", + parents=[parent], # explicitly links as child of the completing task + ) + + ok = kb.complete_task( + conn, parent, + summary="completed with linked child", + created_cards=[other], + ) + assert ok is True + # The card should appear in the completed event's verified_cards list. + import json as _json + row = conn.execute( + "SELECT payload FROM task_events " + "WHERE task_id=? AND kind='completed' ORDER BY id DESC LIMIT 1", + (parent,), + ).fetchone() + payload = _json.loads(row["payload"]) + assert other in payload.get("verified_cards", []) + finally: + conn.close() + + def test_complete_prose_scan_flags_nonexistent_ids(kanban_home): """Successful completion whose summary references a ``t_`` id that doesn't resolve emits a ``suspected_hallucinated_references`` @@ -3184,10 +3237,21 @@ def _signal(pid, sig): ) kb.claim_task(conn, tid) kb._set_worker_pid(conn, tid, os.getpid()) + # Since PR #19473 (salvaged) changed enforce_max_runtime to read + # from task_runs.started_at (per-attempt) rather than + # tasks.started_at (lifetime), we need to backdate BOTH to + # guarantee the timeout fires regardless of which column the + # query pulls from. with kb.write_txn(conn): + long_ago = int(time.time()) - 30 conn.execute( "UPDATE tasks SET started_at = ? WHERE id = ?", - (int(time.time()) - 30, tid), + (long_ago, tid), + ) + conn.execute( + "UPDATE task_runs SET started_at = ? " + "WHERE id = (SELECT current_run_id FROM tasks WHERE id = ?)", + (long_ago, tid), ) before = kb.get_task(conn, tid) assert before.consecutive_failures == 0 diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index e6d25a3d84c..7068e773d1b 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -182,6 +182,52 @@ def test_stale_claim_reclaimed(kanban_home): assert kb.get_task(conn, t).status == "ready" +def test_max_runtime_uses_current_run_start_after_retry(kanban_home): + """A retry should get a fresh max-runtime window. + + ``tasks.started_at`` intentionally records the first time the task ever + started. Runtime enforcement must therefore use the active + ``task_runs.started_at`` row; otherwise every retry of an old task is + immediately timed out again. + """ + with kb.connect() as conn: + host = kb._claimer_id().split(":", 1)[0] + t = kb.create_task( + conn, title="retry", assignee="a", max_runtime_seconds=10, + ) + + kb.claim_task(conn, t, claimer=f"{host}:first") + first_run_id = kb.latest_run(conn, t).id + old_started = int(time.time()) - 20 + conn.execute( + "UPDATE tasks SET started_at = ?, worker_pid = ? WHERE id = ?", + (old_started, 999999, t), + ) + conn.execute( + "UPDATE task_runs SET started_at = ?, worker_pid = ? WHERE id = ?", + (old_started, 999999, first_run_id), + ) + + timed_out = kb.enforce_max_runtime(conn, signal_fn=lambda _pid, _sig: None) + assert timed_out == [t] + assert kb.get_task(conn, t).status == "ready" + + kb.claim_task(conn, t, claimer=f"{host}:retry") + retry_run = kb.latest_run(conn, t) + conn.execute( + "UPDATE tasks SET worker_pid = ? WHERE id = ?", + (999999, t), + ) + conn.execute( + "UPDATE task_runs SET worker_pid = ? WHERE id = ?", + (999999, retry_run.id), + ) + + timed_out = kb.enforce_max_runtime(conn, signal_fn=lambda _pid, _sig: None) + assert timed_out == [] + assert kb.get_task(conn, t).status == "running" + + def test_heartbeat_extends_claim(kanban_home): with kb.connect() as conn: t = kb.create_task(conn, title="x", assignee="a") @@ -776,3 +822,80 @@ def __init__(self, cmd, **kwargs): default_home / "kanban" / "workspaces" ) assert env["HERMES_KANBAN_TASK"] == "t_dispatch_env" + + +# --------------------------------------------------------------------------- +# latest_summary / latest_summaries — surface task_runs.summary handoffs +# --------------------------------------------------------------------------- + +def test_latest_summary_returns_none_when_no_runs(kanban_home): + """A freshly-created task has no runs and therefore no summary.""" + with kb.connect() as conn: + t = kb.create_task(conn, title="fresh", assignee="alice") + assert kb.latest_summary(conn, t) is None + + +def test_latest_summary_returns_summary_after_complete(kanban_home): + """``complete_task(summary=...)`` is the canonical kanban-worker + handoff; ``latest_summary`` must surface it so dashboards/CLI can + render what the worker actually did.""" + handoff = "shipped 3 files, ran tests, opened PR #42" + with kb.connect() as conn: + t = kb.create_task(conn, title="work", assignee="alice") + kb.complete_task(conn, t, summary=handoff) + assert kb.latest_summary(conn, t) == handoff + + +def test_latest_summary_picks_newest_when_multiple_runs(kanban_home): + """When a task has been re-run (block → unblock → complete), the + newest run's summary wins. We unblock to take the task back to + ``ready``, then complete a second time and verify the second + summary surfaces.""" + with kb.connect() as conn: + t = kb.create_task(conn, title="retry", assignee="alice") + kb.complete_task(conn, t, summary="first attempt") + # Move back to ready by direct SQL — block_task / unblock_task + # paths require an active claim, but we just want a second run + # row to exist with a later ended_at. + conn.execute( + "UPDATE tasks SET status='ready', completed_at=NULL WHERE id=?", + (t,), + ) + # Sleep 1s so the second run's ended_at is provably later than + # the first (complete_task uses int(time.time())). + time.sleep(1.05) + kb.complete_task(conn, t, summary="second attempt — final") + assert kb.latest_summary(conn, t) == "second attempt — final" + + +def test_latest_summary_skips_empty_string(kanban_home): + """A run with an empty-string summary should not mask an earlier + populated one — empty strings carry no information.""" + with kb.connect() as conn: + t = kb.create_task(conn, title="t", assignee="alice") + kb.complete_task(conn, t, summary="real handoff") + # Inject a later run with empty summary directly. Workers + # writing "" instead of None is a real shape we want to ignore. + conn.execute( + "INSERT INTO task_runs (task_id, status, started_at, ended_at, " + "outcome, summary) VALUES (?, 'done', ?, ?, 'completed', ?)", + (t, int(time.time()) + 1, int(time.time()) + 2, ""), + ) + conn.commit() + assert kb.latest_summary(conn, t) == "real handoff" + + +def test_latest_summaries_batch_omits_tasks_without_summary(kanban_home): + """``latest_summaries`` is the dashboard's N+1 escape hatch — it + must return only entries for tasks that actually have a summary, + keep the per-task latest, and accept an empty input gracefully.""" + with kb.connect() as conn: + t1 = kb.create_task(conn, title="a", assignee="alice") + t2 = kb.create_task(conn, title="b", assignee="bob") + t3 = kb.create_task(conn, title="c", assignee="carol") + kb.complete_task(conn, t1, summary="alpha") + kb.complete_task(conn, t3, summary="charlie") + out = kb.latest_summaries(conn, [t1, t2, t3]) + assert out == {t1: "alpha", t3: "charlie"} + # Empty input → empty dict, no SQL syntax error from "IN ()". + assert kb.latest_summaries(conn, []) == {} diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index 580b187eccb..b266f0914e5 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -203,7 +203,10 @@ def test_patch_block_then_unblock(client): def test_patch_drag_drop_move_todo_to_ready(client): """Direct status write: the drag-drop path for statuses without a - dedicated verb (e.g. manually promoting todo -> ready).""" + dedicated verb (e.g. manually promoting todo -> ready). + + Promoting a child whose parent is not done is rejected (409). + Promoting a child whose parent IS done is accepted (200).""" parent = client.post("/api/plugins/kanban/tasks", json={"title": "p"}).json()["task"] child = client.post( "/api/plugins/kanban/tasks", @@ -211,12 +214,23 @@ def test_patch_drag_drop_move_todo_to_ready(client): ).json()["task"] assert child["status"] == "todo" + # Rejected: parent not done yet. r = client.patch( f"/api/plugins/kanban/tasks/{child['id']}", json={"status": "ready"}, ) + assert r.status_code == 409 + + # Complete the parent. + r = client.patch( + f"/api/plugins/kanban/tasks/{parent['id']}", + json={"status": "done"}, + ) assert r.status_code == 200 - assert r.json()["task"]["status"] == "ready" + + # Now child auto-promoted by recompute_ready — already ready. + child_after = client.get(f"/api/plugins/kanban/tasks/{child['id']}").json()["task"] + assert child_after["status"] == "ready" def test_patch_reassign(client): @@ -433,13 +447,17 @@ def test_board_progress_rollup(client): "/api/plugins/kanban/tasks", json={"title": "b", "parents": [parent["id"]]}, ).json()["task"] - # Children start as "todo" because the parent isn't done yet; promote - # them to "ready" so complete_task will accept the transition. + # Children start as "todo" because the parent isn't done yet. Set the + # parent to done so children auto-promote to ready via recompute_ready. + r = client.patch( + f"/api/plugins/kanban/tasks/{parent['id']}", + json={"status": "done"}, + ) + assert r.status_code == 200 + # Verify children are now ready. for cid in (child_a["id"], child_b["id"]): - r = client.patch( - f"/api/plugins/kanban/tasks/{cid}", json={"status": "ready"}, - ) - assert r.status_code == 200 + t = client.get(f"/api/plugins/kanban/tasks/{cid}").json()["task"] + assert t["status"] == "ready", f"{cid} should be ready after parent done" # 0/2 done. r = client.get("/api/plugins/kanban/board") @@ -604,6 +622,32 @@ def test_dashboard_done_actions_prompt_for_completion_summary(): assert "body: JSON.stringify(finalPatch)" in bundle +def test_dashboard_dependency_selects_use_value_change_handler(): + """Regression for the dependency selects in the task drawer: the + add-parent / add-child dropdowns must wire through the shared + selectChangeHandler helper so their value actually lands on the + underlying React state. Salvaged from #20019 @LeonSGP43. + """ + repo_root = Path(__file__).resolve().parents[2] + bundle = ( + repo_root / "plugins" / "kanban" / "dashboard" / "dist" / "index.js" + ).read_text() + + parent_select = ( + 'value: newParent,\n' + ' className: "h-7 text-xs flex-1",\n' + ' }, selectChangeHandler(setNewParent))' + ) + child_select = ( + 'value: newChild,\n' + ' className: "h-7 text-xs flex-1",\n' + ' }, selectChangeHandler(setNewChild))' + ) + + assert parent_select in bundle + assert child_select in bundle + + def test_bulk_archive(client): a = client.post("/api/plugins/kanban/tasks", json={"title": "a"}).json()["task"] b = client.post("/api/plugins/kanban/tasks", json={"title": "b"}).json()["task"]