fix(kanban): harden durable artifact handoff

This commit is contained in:
Teknium 2026-07-12 22:50:49 -07:00
parent e6c42b5d80
commit 8030b01a2a
9 changed files with 275 additions and 15 deletions

View file

@ -1072,10 +1072,33 @@ def _profile_cache_roots() -> List[Path]:
return roots
def _kanban_attachment_roots() -> List[Path]:
"""Return durable Kanban attachment roots without importing kanban_db."""
override = os.environ.get("HERMES_KANBAN_ATTACHMENTS_ROOT", "").strip()
if override:
return [Path(override).expanduser()]
home_override = os.environ.get("HERMES_KANBAN_HOME", "").strip()
root = Path(home_override).expanduser() if home_override else _HERMES_ROOT
roots = [root / "kanban" / "attachments"]
boards_root = root / "kanban" / "boards"
try:
board_dirs = [
path for path in boards_root.iterdir()
if path.is_dir() and not path.is_symlink()
and re.fullmatch(r"[a-z0-9][a-z0-9_-]{0,63}", path.name)
and (path / "kanban.db").is_file()
]
except OSError:
return roots
roots.extend(path / "attachments" for path in board_dirs)
return roots
def _media_delivery_allowed_roots() -> List[Path]:
"""Return roots from which model-emitted local media may be delivered."""
roots = [Path(root) for root in MEDIA_DELIVERY_SAFE_ROOTS]
roots.extend(_profile_cache_roots())
roots.extend(_kanban_attachment_roots())
extra_roots = os.environ.get(MEDIA_DELIVERY_ALLOW_DIRS_ENV, "")
for chunk in extra_roots.split(os.pathsep):
for raw_root in chunk.split(","):

View file

@ -135,6 +135,7 @@ BLOCK_RECURRENCE_LIMIT = 2
VALID_WORKSPACE_KINDS = {"scratch", "worktree", "dir"}
KNOWN_TOOLSET_NAMES = frozenset(name.casefold() for name in get_toolset_names())
_IS_WINDOWS = sys.platform == "win32"
KANBAN_ATTACHMENT_MAX_BYTES = 25 * 1024 * 1024
def _fire_kanban_lifecycle_hook(event: str, task_id: str, **fields: Any) -> None:
@ -3975,6 +3976,10 @@ class HallucinatedCardsError(ValueError):
)
class ArtifactPreservationError(RuntimeError):
"""Raised when a declared scratch deliverable cannot be preserved."""
def complete_task(
conn: sqlite3.Connection,
task_id: str,
@ -4042,6 +4047,9 @@ def complete_task(
else:
verified_cards = []
metadata = _merge_completion_prose_artifacts(
conn, task_id, metadata, summary=summary, result=result,
)
with write_txn(conn):
if expected_run_id is None:
cur = conn.execute(
@ -4082,6 +4090,16 @@ def complete_task(
return False
if isinstance(metadata, dict):
_persist_scratch_completion_artifacts(conn, task_id, metadata)
for stored_path in metadata.pop("_staged_artifacts", []):
path = Path(stored_path)
_insert_completion_attachment(
conn,
task_id,
filename=path.name,
stored_path=str(path),
size=path.stat().st_size,
created_at=now,
)
run_id = _end_run(
conn, task_id,
outcome="completed", status="done",
@ -4176,6 +4194,54 @@ def complete_task(
# Workspace / tmux cleanup
# ---------------------------------------------------------------------------
def _merge_completion_prose_artifacts(
conn: sqlite3.Connection,
task_id: str,
metadata: Optional[dict],
*,
summary: Optional[str],
result: Optional[str],
) -> Optional[dict]:
"""Promote existing scratch files named in legacy completion prose.
``artifacts=[...]`` is preferred. Older workers only wrote an absolute
deliverable path in ``summary``/``result``; discover it while scratch still
exists so cleanup cannot erase the file the user was promised.
"""
row = conn.execute(
"SELECT workspace_kind, workspace_path FROM tasks WHERE id = ?",
(task_id,),
).fetchone()
if not row or row["workspace_kind"] != "scratch" or not row["workspace_path"]:
return metadata
workspace = Path(row["workspace_path"]).expanduser()
if not _is_managed_scratch_path(workspace):
return metadata
text = "\n".join(part for part in (summary, result) if part)
if not text:
return metadata
prefix = re.escape(str(workspace))
discovered: list[str] = []
for match in re.finditer(prefix + r"(?:[/\\][^\s`\"'<>]+)", text):
raw = match.group(0).rstrip(".,;:!?)]}")
candidate = Path(raw)
if candidate.is_file():
discovered.append(str(candidate))
if not discovered:
return metadata
updated = dict(metadata) if isinstance(metadata, dict) else {}
existing = updated.get("artifacts")
merged = list(existing) if isinstance(existing, (list, tuple)) else []
seen = {str(path) for path in merged}
for path in discovered:
if path not in seen:
merged.append(path)
seen.add(path)
updated["artifacts"] = merged
return updated
def _persist_scratch_completion_artifacts(
conn: sqlite3.Connection,
task_id: str,
@ -4208,6 +4274,17 @@ def _persist_scratch_completion_artifacts(
used_destinations: set[Path] = set()
changed = False
def _discard_copies() -> None:
for copied in used_destinations:
try:
copied.unlink(missing_ok=True)
except OSError:
pass
try:
attachment_dir.rmdir()
except OSError:
pass
for item in raw_artifacts:
artifact = str(item).strip() if isinstance(item, str) else ""
if not artifact:
@ -4219,29 +4296,83 @@ def _persist_scratch_completion_artifacts(
persisted.append(artifact)
continue
if not src.is_file() or not resolved_src.is_relative_to(workspace_root):
if not resolved_src.is_relative_to(workspace_root):
persisted.append(artifact)
continue
if not src.is_file():
_discard_copies()
raise ArtifactPreservationError(
f"declared scratch artifact is unavailable or not a regular file: {artifact}"
)
size = resolved_src.stat().st_size
if size > KANBAN_ATTACHMENT_MAX_BYTES:
_discard_copies()
raise ArtifactPreservationError(
f"declared scratch artifact exceeds the "
f"{KANBAN_ATTACHMENT_MAX_BYTES}-byte limit: {artifact}"
)
dest: Optional[Path] = None
try:
attachment_dir.mkdir(parents=True, exist_ok=True)
dest = _unique_attachment_path(attachment_dir, resolved_src.name, used_destinations)
shutil.copy2(resolved_src, dest)
with resolved_src.open("rb") as source_file, dest.open("xb") as destination_file:
copied = 0
while chunk := source_file.read(1024 * 1024):
copied += len(chunk)
if copied > KANBAN_ATTACHMENT_MAX_BYTES:
raise ArtifactPreservationError(
f"declared scratch artifact grew beyond the size limit: {artifact}"
)
destination_file.write(chunk)
except Exception as exc:
_log.debug(
"Failed to persist completion artifact for task %s: %s",
task_id,
exc,
)
persisted.append(artifact)
continue
if dest is not None:
try:
dest.unlink(missing_ok=True)
except OSError:
pass
_discard_copies()
if isinstance(exc, ArtifactPreservationError):
raise
raise ArtifactPreservationError(
f"could not preserve declared scratch artifact {artifact}: {exc}"
) from exc
used_destinations.add(dest)
persisted.append(str(dest))
persisted.append(str(dest.resolve()))
changed = True
if changed:
metadata["artifacts"] = persisted
metadata["_staged_artifacts"] = [
path for path in persisted if path.startswith(str(attachment_dir.resolve()))
]
def _insert_completion_attachment(
conn: sqlite3.Connection,
task_id: str,
*,
filename: str,
stored_path: str,
size: int,
created_at: int,
) -> None:
"""Record a worker-produced artifact in the existing attachment table."""
conn.execute(
"INSERT INTO task_attachments "
"(task_id, filename, stored_path, content_type, size, uploaded_by, created_at) "
"VALUES (?, ?, ?, NULL, ?, 'kanban_complete', ?)",
(task_id, filename, stored_path, size, created_at),
)
_append_event(
conn,
task_id,
"attached",
{"filename": filename, "size": size, "by": "kanban_complete"},
)
def _unique_attachment_path(directory: Path, filename: str, used: set[Path]) -> Path:

View file

@ -646,7 +646,7 @@ def create_task(payload: CreateTaskBody, board: Optional[str] = Query(None)):
# Cap a single upload so a runaway request can't fill the disk. 25 MB
# comfortably covers PDFs, images, and source docs — the kanban use case.
_MAX_ATTACHMENT_BYTES = 25 * 1024 * 1024
_MAX_ATTACHMENT_BYTES = kanban_db.KANBAN_ATTACHMENT_MAX_BYTES
def _safe_attachment_name(raw: str) -> str:

View file

@ -786,6 +786,28 @@ class TestMediaDeliveryPathValidation:
assert BasePlatformAdapter.validate_media_delivery_path(str(media_file)) == str(media_file.resolve())
def test_allows_stale_kanban_attachment_but_not_neighboring_workspace(
self, tmp_path, monkeypatch,
):
"""Strict mode trusts durable attachments without trusting scratch."""
self._patch_roots(monkeypatch)
monkeypatch.setenv("HERMES_KANBAN_HOME", str(tmp_path / "hermes"))
monkeypatch.setenv("HERMES_MEDIA_TRUST_RECENT_FILES", "0")
board_root = tmp_path / "hermes" / "kanban" / "boards" / "research"
board_root.mkdir(parents=True)
(board_root / "kanban.db").touch()
attachment = board_root / "attachments" / "t_12345678" / "report.pdf"
scratch = board_root / "workspaces" / "t_12345678" / "notes.txt"
attachment.parent.mkdir(parents=True)
scratch.parent.mkdir(parents=True)
attachment.write_bytes(b"%PDF")
scratch.write_text("private", encoding="utf-8")
assert BasePlatformAdapter.validate_media_delivery_path(str(attachment)) == str(
attachment.resolve()
)
assert BasePlatformAdapter.validate_media_delivery_path(str(scratch)) is None
def test_recency_trust_allows_freshly_produced_file(self, tmp_path, monkeypatch):
"""A PDF the agent just wrote to /tmp should be deliverable.

View file

@ -2392,6 +2392,56 @@ def test_complete_task_persists_scratch_artifacts_before_cleanup(kanban_home):
assert str(persisted) != str(artifact)
assert run is not None
assert run.metadata["artifacts"] == [str(persisted)]
with kb.connect() as conn:
attachments = kb.list_attachments(conn, t)
assert [(a.filename, a.stored_path) for a in attachments] == [
("chart.png", str(persisted.resolve()))
]
def test_complete_task_rejects_missing_declared_scratch_artifact(kanban_home):
"""A declared scratch deliverable must not disappear behind a false Done."""
with kb.connect() as conn:
t = kb.create_task(conn, title="missing report")
task = kb.get_task(conn, t)
ws = kb.resolve_workspace(task)
kb.set_workspace_path(conn, t, ws)
missing = ws / "report.md"
with pytest.raises(kb.ArtifactPreservationError, match="unavailable"):
kb.complete_task(
conn,
t,
result="report complete",
metadata={"artifacts": [str(missing)]},
)
assert kb.get_task(conn, t).status == "ready"
assert kb.list_attachments(conn, t) == []
assert ws.exists(), "failed completion must keep scratch available for retry"
def test_complete_task_preserves_legacy_artifact_path_from_summary(kanban_home):
"""Summary-only workers keep the file they tell the user was delivered."""
with kb.connect() as conn:
t = kb.create_task(conn, title="legacy report")
task = kb.get_task(conn, t)
ws = kb.resolve_workspace(task)
kb.set_workspace_path(conn, t, ws)
report = ws / "report.md"
report.write_text("legacy deliverable", encoding="utf-8")
assert kb.complete_task(
conn,
t,
summary=f"Task complete — delivered {report}",
)
run = kb.latest_run(conn, t)
persisted = Path(run.metadata["artifacts"][0])
assert not ws.exists()
assert persisted.read_text(encoding="utf-8") == "legacy deliverable"
assert persisted.parent == kb.task_attachments_dir(t)
def test_complete_task_leaves_non_scratch_artifact_paths_unchanged(

View file

@ -484,6 +484,31 @@ def test_complete_rejects_non_list_artifacts(worker_env):
assert "artifacts must be a list" in err
def test_complete_missing_scratch_artifact_stays_in_flight(worker_env):
"""A false deliverable claim must return retry guidance, not mark Done."""
from hermes_cli import kanban_db as kb
from tools import kanban_tools as kt
with kb.connect() as conn:
task = kb.get_task(conn, worker_env)
assert task is not None
workspace = kb.resolve_workspace(task)
kb.set_workspace_path(conn, worker_env, workspace)
output = kt._handle_complete({
"summary": "report complete",
"artifacts": [str(workspace / "missing-report.md")],
})
error = json.loads(output).get("error", "")
assert "could not preserve" in error
assert "still in-flight" in error
assert "retry kanban_complete" in error
with kb.connect() as conn:
assert kb.get_task(conn, worker_env).status == "running"
assert workspace.exists()
def test_complete_rejects_no_handoff(worker_env):
from tools import kanban_tools as kt
out = kt._handle_complete({})

View file

@ -630,6 +630,13 @@ def _handle_complete(args: dict, **kw) -> str:
created_cards=created_cards,
expected_run_id=_worker_run_id(tid),
)
except kb.ArtifactPreservationError as artifact_err:
return tool_error(
f"kanban_complete could not preserve the declared artifacts: "
f"{artifact_err}. Your task is still in-flight and its "
f"scratch workspace was kept. Fix the artifact path or "
f"storage error, then retry kanban_complete with the same handoff."
)
except kb.HallucinatedCardsError as hall_err:
# Structured rejection — surface the phantom ids so the
# worker can retry with a corrected list or drop the
@ -1277,8 +1284,10 @@ KANBAN_COMPLETE_SCHEMA = {
"lands with the completion notification. Skip "
"intermediate scratch files and references that "
"are not the deliverable. The path must exist "
"on disk when the notifier runs; missing files "
"are silently skipped."
"on disk at completion. Files inside a managed scratch "
"workspace are copied to durable task attachments before "
"cleanup; a missing declared scratch artifact keeps the "
"task in-flight so you can fix the path and retry."
),
},
"board": _board_schema_prop(),

View file

@ -63,7 +63,7 @@ They coexist: a kanban worker may call `delegate_task` internally during its run
- **Link**`task_links` row recording a parent → child dependency. The dispatcher promotes `todo → ready` when all parents are `done`.
- **Comment** — the inter-agent protocol. Agents and humans append comments; when a worker is (re-)spawned it reads the full comment thread as part of its context.
- **Workspace** — the directory a worker operates in. Three kinds:
- `scratch` (default) — fresh tmp dir under `~/.hermes/kanban/workspaces/<id>/` (or `~/.hermes/kanban/boards/<slug>/workspaces/<id>/` on non-default boards). **Deleted when the task completes** — scratch is ephemeral by design, so the dir is wiped the moment the worker (or `hermes kanban complete <id>`) marks the task done. If you want to keep the worker's output, use `worktree:` or `dir:<path>` instead. The first time a scratch workspace is created on an install, the dispatcher logs a warning and emits a `tip_scratch_workspace` event on the task (visible via `hermes kanban show <id>`).
- `scratch` (default) — fresh tmp dir under `~/.hermes/kanban/workspaces/<id>/` (or `~/.hermes/kanban/boards/<slug>/workspaces/<id>/` on non-default boards). **Deleted when the task completes** — scratch is ephemeral by design. Files explicitly declared through `kanban_complete(artifacts=[...])` are copied into durable per-task attachment storage before cleanup; existing deliverable paths in legacy completion summaries receive the same treatment. Other scratch files are removed. A missing declared scratch artifact keeps the task in-flight so the worker can correct the path and retry. Use `worktree:` or `dir:<path>` when the whole workspace should remain available. The first time a scratch workspace is created on an install, the dispatcher logs a warning and emits a `tip_scratch_workspace` event on the task (visible via `hermes kanban show <id>`).
- `dir:<path>` — an existing shared directory (Obsidian vault, mail ops dir, per-account folder). **Must be an absolute path.** Relative paths like `dir:../tenants/foo/` are rejected at dispatch because they'd resolve against whatever CWD the dispatcher happens to be in, which is ambiguous and a confused-deputy escape vector. The path is otherwise trusted — it's your box, your filesystem, the worker runs with your uid. This is the trusted-local-user threat model; kanban is single-host by design. **Preserved on completion.**
- `worktree` — a git worktree under `.worktrees/<id>/` for coding tasks. Use `worktree:<path>` to pin the exact target path. Worker-side `git worktree add` creates it, using `--branch` when provided. **Preserved on completion.**
- **Dispatcher** — a long-lived loop that, every N seconds (default 60): reclaims stale claims, reclaims crashed workers (PID gone but TTL not yet expired), promotes ready tasks, atomically claims, spawns assigned profiles. Runs **inside the gateway** by default (`kanban.dispatch_in_gateway: true`). One dispatcher sweeps all boards per tick; workers are spawned with `HERMES_KANBAN_BOARD` pinned so they can't see other boards. After `kanban.failure_limit` consecutive spawn failures on the same task (default: 2) the dispatcher auto-blocks it with the last error as the reason — prevents thrashing on tasks whose profile doesn't exist, workspace can't mount, etc.

View file

@ -59,7 +59,7 @@ Hermes Kanban 是一个持久化任务看板,在所有 Hermes 配置文件之
- **Link链接** —— `task_links` 行,记录父 → 子依赖关系。当所有父任务变为 `done` 时,调度器将 `todo → ready`
- **Comment评论** —— agent 间协议。Agent 和人类追加评论;当 worker 被(重新)启动时,它将完整的评论线程作为上下文的一部分读取。
- **Workspace工作区** —— worker 操作的目录。三种类型:
- `scratch`(默认)—— 在 `~/.hermes/kanban/workspaces/<id>/` 下(非默认看板为 `~/.hermes/kanban/boards/<slug>/workspaces/<id>/`)创建的临时目录。**任务完成时删除** —— scratch 是临时性的worker`hermes kanban complete <id>`)将任务标记为完成的那一刻,目录即被清除。如果想保留 worker 的输出,请使用 `worktree:``dir:<path>`。在某次安装中首次创建 scratch 工作区时,调度器会记录警告并在任务上发出 `tip_scratch_workspace` 事件(可通过 `hermes kanban show <id>` 查看)。
- `scratch`(默认)—— 在 `~/.hermes/kanban/workspaces/<id>/` 下(非默认看板为 `~/.hermes/kanban/boards/<slug>/workspaces/<id>/`)创建的临时目录。**任务完成时删除** —— scratch 按设计是临时性的。通过 `kanban_complete(artifacts=[...])` 明确声明的文件会在清理前复制到持久的任务附件存储;旧版完成摘要中已存在的交付文件路径也会得到同样处理。其他 scratch 文件仍会被删除。如果声明的 scratch 交付文件不存在任务会保持进行中worker 可修正路径后重试。需要保留整个工作区时,请使用 `worktree:``dir:<path>`。在某次安装中首次创建 scratch 工作区时,调度器会记录警告并在任务上发出 `tip_scratch_workspace` 事件(可通过 `hermes kanban show <id>` 查看)。
- `dir:<path>` —— 现有的共享目录Obsidian vault、邮件运维目录、每账号文件夹。**必须是绝对路径。** 像 `dir:../tenants/foo/` 这样的相对路径在调度时会被拒绝,因为它们会相对于调度器碰巧所在的 CWD 解析这是模糊的也是混淆代理confused-deputy逃逸向量。路径本身是受信任的 —— 这是你的机器、你的文件系统worker 以你的 uid 运行。这是受信任本地用户的威胁模型kanban 设计为单主机。**完成时保留。**
- `worktree` —— 用于编码任务的 git worktree位于 `.worktrees/<id>/` 下。使用 `worktree:<path>` 固定确切的目标路径。Worker 端的 `git worktree add` 创建它,提供 `--branch` 时使用该分支。**完成时保留。**
- **Dispatcher调度器** —— 一个长期运行的循环,每 N 秒(默认 60 秒)执行一次:回收过期的认领、回收崩溃的 workerPID 消失但 TTL 尚未过期)、推进就绪任务、原子性认领、启动已分配的配置文件。默认**在 gateway 内部运行**`kanban.dispatch_in_gateway: true`)。每次 tick 一个调度器扫描所有看板worker 启动时固定了 `HERMES_KANBAN_BOARD`,因此无法看到其他看板。在同一任务上连续启动失败 `kanban.failure_limit`默认2调度器会以最后一个错误为原因自动阻塞该任务 —— 防止因配置文件不存在、工作区无法挂载等原因导致的反复抖动。