mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
fix(kanban): accept created_cards linked as child of completing task
Widens _verify_created_cards to also accept ids that are children of the completing task in task_links. Previously we only accepted cards where created_by matched the completing task's assignee, which was too strict for legitimate orchestrator flows: a specifier creates a card (so created_by=specifier, not worker), then a worker picks it up and passes parents=[current_task] to kanban_create. The explicit link proves the relationship and should be trusted. Salvaged from #20022 @LeonSGP43 (full PR superseded by #20232 + this patch; the linked-children relaxation was the portable improvement).
This commit is contained in:
parent
eda326df16
commit
6d302b340e
2 changed files with 63 additions and 10 deletions
|
|
@ -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).
|
||||
A card is "verified" iff a row exists in ``tasks`` AND at least one
|
||||
of the following holds:
|
||||
|
||||
``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.
|
||||
* ``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=[<current_task>]``. 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
|
||||
|
|
|
|||
|
|
@ -2978,6 +2978,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_<hex>`` id
|
||||
that doesn't resolve emits a ``suspected_hallucinated_references``
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue