fix(kanban): isolate review handoff ownership
This commit is contained in:
committed by
Teknium
parent
6d7e86c262
commit
0acf49b16f
@@ -18248,6 +18248,13 @@ def _run_kanban_goal_loop_q(cli: "HermesCLI", first_response: str) -> None:
|
||||
task_id = (_os.environ.get("HERMES_KANBAN_TASK") or "").strip()
|
||||
if not task_id:
|
||||
return
|
||||
worker_run_id = None
|
||||
raw_run_id = (_os.environ.get("HERMES_KANBAN_RUN_ID") or "").strip()
|
||||
if raw_run_id:
|
||||
try:
|
||||
worker_run_id = int(raw_run_id)
|
||||
except ValueError:
|
||||
logger.warning("invalid HERMES_KANBAN_RUN_ID=%r", raw_run_id)
|
||||
|
||||
from hermes_cli import kanban_db as _kb
|
||||
from hermes_cli.goals import run_kanban_goal_loop as _run_loop, DEFAULT_MAX_TURNS as _DEF_TURNS
|
||||
@@ -18293,18 +18300,7 @@ def _run_kanban_goal_loop_q(cli: "HermesCLI", first_response: str) -> None:
|
||||
def _task_status() -> "str | None":
|
||||
c = _kb.connect()
|
||||
try:
|
||||
t = _kb.get_task(c, task_id)
|
||||
if t is None:
|
||||
return None
|
||||
# ``request_changes`` deliberately lands back in ready/todo. For a
|
||||
# goal-mode reviewer that is nevertheless a terminal outcome for
|
||||
# this process; otherwise the review agent would keep consuming
|
||||
# implementation turns after it released its claim.
|
||||
if t.status in {"ready", "todo"}:
|
||||
events = _kb.list_events(c, task_id)
|
||||
if events and events[-1].kind == "changes_requested":
|
||||
return "changes_requested"
|
||||
return t.status
|
||||
return _kb.goal_run_status(c, task_id, worker_run_id)
|
||||
finally:
|
||||
try:
|
||||
c.close()
|
||||
@@ -18314,7 +18310,12 @@ def _run_kanban_goal_loop_q(cli: "HermesCLI", first_response: str) -> None:
|
||||
def _block(reason: str) -> None:
|
||||
c = _kb.connect()
|
||||
try:
|
||||
_kb.block_task(c, task_id, reason=reason)
|
||||
_kb.block_task(
|
||||
c,
|
||||
task_id,
|
||||
reason=reason,
|
||||
expected_run_id=worker_run_id,
|
||||
)
|
||||
finally:
|
||||
try:
|
||||
c.close()
|
||||
|
||||
@@ -2467,22 +2467,22 @@ def _cmd_reopen_review(args: argparse.Namespace) -> int:
|
||||
return 1
|
||||
reason = getattr(args, "reason", None)
|
||||
if reason is not None:
|
||||
reason = reason.strip() or None
|
||||
reason = str(kb.redact_review_value(reason.strip())).strip() or None
|
||||
author = _profile_author() if reason else None
|
||||
failed: list[str] = []
|
||||
with kb.connect_closing() as conn:
|
||||
for tid in ids:
|
||||
if reason:
|
||||
kb.add_comment(
|
||||
conn,
|
||||
tid,
|
||||
author or "operator",
|
||||
f"CHANGES REQUESTED: {reason}",
|
||||
)
|
||||
if not kb.reopen_review_task(conn, tid):
|
||||
failed.append(tid)
|
||||
print(f"cannot reopen {tid} (not in review?)", file=sys.stderr)
|
||||
else:
|
||||
if reason:
|
||||
kb.add_comment(
|
||||
conn,
|
||||
tid,
|
||||
author or "operator",
|
||||
f"CHANGES REQUESTED: {reason}",
|
||||
)
|
||||
print(f"Reopened {tid}" + (f": {reason}" if reason else ""))
|
||||
return 0 if not failed else 1
|
||||
|
||||
|
||||
+58
-7
@@ -4579,6 +4579,57 @@ def _retry_status_for_run(
|
||||
return "review" if payload.get("source_status") == "review" else "ready"
|
||||
|
||||
|
||||
def goal_run_status(
|
||||
conn: sqlite3.Connection,
|
||||
task_id: str,
|
||||
expected_run_id: Optional[int] = None,
|
||||
) -> Optional[str]:
|
||||
"""Resolve lifecycle status from the perspective of one worker run.
|
||||
|
||||
A successor may claim the task immediately after this run hands it off.
|
||||
Returning the task's live ``running`` status in that case lets the old goal
|
||||
loop mutate the successor. Bind terminal handoffs to the original run and
|
||||
report any other ownership loss as ``superseded``.
|
||||
"""
|
||||
task = get_task(conn, task_id)
|
||||
if task is None:
|
||||
return None
|
||||
if expected_run_id is not None:
|
||||
row = conn.execute(
|
||||
"SELECT outcome FROM task_runs WHERE id = ? AND task_id = ?",
|
||||
(int(expected_run_id), task_id),
|
||||
).fetchone()
|
||||
outcome = (
|
||||
str(row["outcome"])
|
||||
if row and row["outcome"] is not None
|
||||
else None
|
||||
)
|
||||
terminal_status = (
|
||||
{
|
||||
"completed": "done",
|
||||
"review_requested": "review",
|
||||
"changes_requested": "changes_requested",
|
||||
"blocked": "blocked",
|
||||
"dependency_wait": "blocked",
|
||||
}.get(outcome)
|
||||
if outcome is not None
|
||||
else None
|
||||
)
|
||||
if terminal_status is not None:
|
||||
return terminal_status
|
||||
if outcome is not None or task.current_run_id != int(expected_run_id):
|
||||
return "superseded"
|
||||
if task.status in {"ready", "todo"}:
|
||||
event = conn.execute(
|
||||
"SELECT kind FROM task_events WHERE task_id = ? "
|
||||
"ORDER BY id DESC LIMIT 1",
|
||||
(task_id,),
|
||||
).fetchone()
|
||||
if event and event["kind"] == "changes_requested":
|
||||
return "changes_requested"
|
||||
return task.status
|
||||
|
||||
|
||||
def heartbeat_claim(
|
||||
conn: sqlite3.Connection,
|
||||
task_id: str,
|
||||
@@ -6038,18 +6089,18 @@ def block_task(
|
||||
|
||||
|
||||
|
||||
def _redact_review_value(value: Any) -> Any:
|
||||
def redact_review_value(value: Any) -> Any:
|
||||
"""Redact secrets at the domain boundary for durable review handoffs."""
|
||||
if isinstance(value, str):
|
||||
from agent.redact import redact_sensitive_text
|
||||
|
||||
return redact_sensitive_text(value, force=True)
|
||||
if isinstance(value, dict):
|
||||
return {key: _redact_review_value(item) for key, item in value.items()}
|
||||
return {key: redact_review_value(item) for key, item in value.items()}
|
||||
if isinstance(value, list):
|
||||
return [_redact_review_value(item) for item in value]
|
||||
return [redact_review_value(item) for item in value]
|
||||
if isinstance(value, tuple):
|
||||
return tuple(_redact_review_value(item) for item in value)
|
||||
return tuple(redact_review_value(item) for item in value)
|
||||
return value
|
||||
|
||||
|
||||
@@ -6070,8 +6121,8 @@ def request_review(
|
||||
right profile. Supplying ``reviewer`` also reassigns the task before it is
|
||||
exposed to the review dispatcher.
|
||||
"""
|
||||
summary = _redact_review_value(summary)
|
||||
metadata = _redact_review_value(metadata)
|
||||
summary = redact_review_value(summary)
|
||||
metadata = redact_review_value(metadata)
|
||||
with write_txn(conn):
|
||||
if not _parents_satisfied(conn, task_id):
|
||||
return False
|
||||
@@ -6156,7 +6207,7 @@ def request_changes(
|
||||
``changes_requested`` event. The second tuple item is the implementer on
|
||||
success or a diagnostic reason on failure.
|
||||
"""
|
||||
reason = str(_redact_review_value(reason or "")).strip()
|
||||
reason = str(redact_review_value(reason or "")).strip()
|
||||
if not reason:
|
||||
return False, "reason is required"
|
||||
|
||||
|
||||
@@ -875,8 +875,14 @@ def update_task(task_id: str, payload: UpdateTaskBody, board: Optional[str] = Qu
|
||||
if task is None:
|
||||
raise HTTPException(status_code=404, detail=f"task {task_id} not found")
|
||||
|
||||
review_assignee_deferred = (
|
||||
payload.status == "review" and payload.assignee is not None
|
||||
)
|
||||
|
||||
# --- assignee ----------------------------------------------------
|
||||
if payload.assignee is not None:
|
||||
# For a combined assignee+review patch, request_review must capture
|
||||
# the current implementer before routing the task to the reviewer.
|
||||
if payload.assignee is not None and not review_assignee_deferred:
|
||||
try:
|
||||
ok = kanban_db.assign_task(
|
||||
conn, task_id, payload.assignee or None,
|
||||
@@ -909,7 +915,10 @@ def update_task(task_id: str, payload: UpdateTaskBody, board: Optional[str] = Qu
|
||||
ok = kanban_db.request_review(
|
||||
conn, task_id, summary=payload.summary,
|
||||
metadata=payload.metadata,
|
||||
reviewer=(payload.assignee or None),
|
||||
)
|
||||
if ok and review_assignee_deferred and not payload.assignee:
|
||||
ok = kanban_db.assign_task(conn, task_id, None)
|
||||
elif s == "ready":
|
||||
# Re-open a blocked/scheduled/review task, or just an explicit
|
||||
# status set. "Changes requested" (review -> ready) goes through
|
||||
@@ -1370,6 +1379,7 @@ def bulk_update(payload: BulkTaskBody, board: Optional[str] = Query(None)):
|
||||
ok = kanban_db.request_review(
|
||||
conn, tid, summary=payload.summary,
|
||||
metadata=payload.metadata,
|
||||
reviewer=(payload.assignee or None),
|
||||
)
|
||||
elif s == "ready":
|
||||
cur = kanban_db.get_task(conn, tid)
|
||||
|
||||
@@ -434,6 +434,49 @@ def test_crashed_and_timed_out_review_runs_retry_in_review_phase(
|
||||
assert crashed.status == "review"
|
||||
|
||||
|
||||
def test_goal_run_status_is_bound_to_original_run(conn) -> None:
|
||||
task_id = kb.create_task(conn, title="Goal handoff race", assignee="builder")
|
||||
implementation = kb.claim_task(conn, task_id)
|
||||
assert implementation is not None
|
||||
assert kb.request_review(
|
||||
conn,
|
||||
task_id,
|
||||
summary="ready",
|
||||
reviewer="reviewer",
|
||||
expected_run_id=implementation.current_run_id,
|
||||
)
|
||||
review = kb.claim_review_task(conn, task_id)
|
||||
assert review is not None
|
||||
assert kb.goal_run_status(
|
||||
conn, task_id, implementation.current_run_id
|
||||
) == "review"
|
||||
|
||||
assert kb.request_changes(
|
||||
conn,
|
||||
task_id,
|
||||
reason="fix it",
|
||||
expected_run_id=review.current_run_id,
|
||||
) == (True, "builder")
|
||||
successor = kb.claim_task(conn, task_id)
|
||||
assert successor is not None
|
||||
assert kb.goal_run_status(
|
||||
conn, task_id, review.current_run_id
|
||||
) == "changes_requested"
|
||||
assert kb.goal_run_status(
|
||||
conn, task_id, successor.current_run_id
|
||||
) == "running"
|
||||
assert not kb.block_task(
|
||||
conn,
|
||||
task_id,
|
||||
reason="stale reviewer must not block successor",
|
||||
expected_run_id=review.current_run_id,
|
||||
)
|
||||
current = kb.get_task(conn, task_id)
|
||||
assert current is not None
|
||||
assert current.status == "running"
|
||||
assert current.current_run_id == successor.current_run_id
|
||||
|
||||
|
||||
def test_parked_review_approval_without_evidence_still_creates_audit_run(conn) -> None:
|
||||
task_id = kb.create_task(conn, title="Manual approval", assignee="reviewer")
|
||||
assert kb.request_review(conn, task_id, summary="implementation handoff")
|
||||
|
||||
@@ -248,6 +248,40 @@ def test_worker_guidance_distinguishes_same_card_and_downstream_review() -> None
|
||||
assert "escalate" in skill_text.lower()
|
||||
|
||||
|
||||
def test_cli_reopen_review_is_transition_first_and_redacts_reason(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
secret = "ghp_" + "Q" * 40
|
||||
with kb.connect() as conn:
|
||||
invalid_id = kb.create_task(conn, title="not review", assignee="builder")
|
||||
review_id = kb.create_task(conn, title="review", assignee="builder")
|
||||
assert kb.request_review(conn, review_id, summary="ready")
|
||||
|
||||
invalid_output = kc.run_slash(
|
||||
f'reopen-review {invalid_id} --reason "invalid {secret}"'
|
||||
)
|
||||
assert "cannot reopen" in invalid_output
|
||||
with kb.connect() as conn:
|
||||
assert kb.list_comments(conn, invalid_id) == []
|
||||
|
||||
success_output = kc.run_slash(
|
||||
f'reopen-review {review_id} --reason "revise {secret}"'
|
||||
)
|
||||
assert "Reopened" in success_output
|
||||
assert secret not in success_output
|
||||
with kb.connect() as conn:
|
||||
task = kb.get_task(conn, review_id)
|
||||
assert task is not None
|
||||
assert task.status == "ready"
|
||||
comments = kb.list_comments(conn, review_id)
|
||||
assert len(comments) == 1
|
||||
assert secret not in comments[0].body
|
||||
|
||||
|
||||
def test_goal_mode_review_handoff_cannot_bypass_judge(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
tmp_path: Path,
|
||||
|
||||
@@ -235,6 +235,7 @@ def test_patch_review_lifecycle_preserves_handoff_and_reopens(client):
|
||||
f"/api/plugins/kanban/tasks/{task['id']}",
|
||||
json={
|
||||
"status": "review",
|
||||
"assignee": "reviewer",
|
||||
"summary": f"Implementation ready. {secret}",
|
||||
"metadata": {"tests_run": 4, "token": secret},
|
||||
},
|
||||
@@ -254,7 +255,9 @@ def test_patch_review_lifecycle_preserves_handoff_and_reopens(client):
|
||||
if event.kind == "review_requested"
|
||||
][-1]
|
||||
assert secret not in json.dumps(review_event.payload)
|
||||
assert kb.assign_task(conn, task["id"], "reviewer")
|
||||
assert review_event.payload is not None
|
||||
assert review_event.payload["implementer"] == "builder"
|
||||
assert review_event.payload["reviewer"] == "reviewer"
|
||||
|
||||
response = client.patch(
|
||||
f"/api/plugins/kanban/tasks/{task['id']}",
|
||||
@@ -599,21 +602,59 @@ def test_bulk_status_ready(client):
|
||||
c2 = client.post("/api/plugins/kanban/tasks", json={"title": "c"}).json()["task"]
|
||||
# Parent-less tasks land in "ready" already; push them to blocked first.
|
||||
for tid in (a["id"], b["id"], c2["id"]):
|
||||
client.patch(f"/api/plugins/kanban/tasks/{tid}",
|
||||
json={"status": "blocked", "block_reason": "wait"})
|
||||
client.patch(
|
||||
f"/api/plugins/kanban/tasks/{tid}",
|
||||
json={"status": "blocked", "block_reason": "wait"},
|
||||
)
|
||||
|
||||
r = client.post("/api/plugins/kanban/tasks/bulk",
|
||||
json={"ids": [a["id"], b["id"], c2["id"]], "status": "ready"})
|
||||
assert r.status_code == 200
|
||||
results = r.json()["results"]
|
||||
assert all(r["ok"] for r in results)
|
||||
response = client.post(
|
||||
"/api/plugins/kanban/tasks/bulk",
|
||||
json={"ids": [a["id"], b["id"], c2["id"]], "status": "ready"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
results = response.json()["results"]
|
||||
assert all(item["ok"] for item in results)
|
||||
# All three are now ready.
|
||||
board = client.get("/api/plugins/kanban/board").json()
|
||||
ready = next(col for col in board["columns"] if col["name"] == "ready")
|
||||
ids = {t["id"] for t in ready["tasks"]}
|
||||
ids = {task["id"] for task in ready["tasks"]}
|
||||
assert {a["id"], b["id"], c2["id"]}.issubset(ids)
|
||||
|
||||
|
||||
def test_bulk_review_assignment_preserves_implementer_provenance(client):
|
||||
tasks = [
|
||||
client.post(
|
||||
"/api/plugins/kanban/tasks",
|
||||
json={"title": title, "assignee": "builder"},
|
||||
).json()["task"]
|
||||
for title in ("review a", "review b")
|
||||
]
|
||||
response = client.post(
|
||||
"/api/plugins/kanban/tasks/bulk",
|
||||
json={
|
||||
"ids": [task["id"] for task in tasks],
|
||||
"status": "review",
|
||||
"assignee": "reviewer",
|
||||
"summary": "ready",
|
||||
},
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert all(item["ok"] for item in response.json()["results"])
|
||||
with kb.connect() as conn:
|
||||
for task in tasks:
|
||||
current = kb.get_task(conn, task["id"])
|
||||
assert current is not None
|
||||
assert current.status == "review"
|
||||
assert current.assignee == "reviewer"
|
||||
event = [
|
||||
item for item in kb.list_events(conn, task["id"])
|
||||
if item.kind == "review_requested"
|
||||
][-1]
|
||||
assert event.payload is not None
|
||||
assert event.payload["implementer"] == "builder"
|
||||
assert event.payload["reviewer"] == "reviewer"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# /config endpoint
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -149,56 +149,36 @@ The dashboard view, filtered by `auth-project`:
|
||||
|
||||

|
||||
|
||||
Three-stage chain visible at once: `Spec: password reset flow` (DONE, pm), `Implement password reset flow` (DONE, backend-dev), `Review password reset PR` (READY, reviewer). Each has its parent in green at the bottom and children as dependencies.
|
||||
The screenshot uses the **pre-created downstream card** model: the implementation card has a dedicated reviewer child. In that model the engineer must call `kanban_complete` when implementation is ready so the reviewer child can leave `todo`. Never block the implementation parent merely to ask for review.
|
||||
|
||||
The interesting one is the implementation task, because it was blocked and retried. Here's the full three-agent choreography, shown as the tool calls each worker's model makes:
|
||||
For workflows where the same card owns implementation and review, use the first-class review lifecycle instead. The full implement → review → changes → re-review choreography is:
|
||||
|
||||
```python
|
||||
# --- PM worker spawns on $SPEC and writes the acceptance criteria ---
|
||||
# worker tool calls
|
||||
# --- Engineer: first implementation attempt ---
|
||||
kanban_show()
|
||||
kanban_complete(
|
||||
summary="spec approved; POST /forgot-password sends email, "
|
||||
"GET /reset/:token renders form, POST /reset applies new password",
|
||||
metadata={"acceptance": [
|
||||
"expired token returns 410",
|
||||
"reused last-3 password returns 400 with message",
|
||||
"successful reset invalidates all active sessions",
|
||||
]},
|
||||
# (write code, run tests, prepare the candidate)
|
||||
kanban_request_review(
|
||||
summary="implemented reset flow; candidate is ready for review",
|
||||
metadata={"changed_files": ["auth/reset.py"], "tests_run": 8},
|
||||
reviewer="reviewer",
|
||||
)
|
||||
# → $SPEC is done; $IMPL auto-promotes from todo to ready
|
||||
# → the same card enters review; the implementation run closes as
|
||||
# outcome='review_requested'
|
||||
|
||||
# --- Engineer worker spawns on $IMPL (first attempt) ---
|
||||
# worker tool calls
|
||||
kanban_show() # reads $SPEC's summary + acceptance metadata in worker_context
|
||||
# (engineer writes code, runs tests, opens PR)
|
||||
# Reviewer feedback arrives — engineer decides the concerns are valid and blocks
|
||||
kanban_block(
|
||||
reason="Review: password strength check missing, reset link isn't "
|
||||
"single-use (can be replayed within 30min)",
|
||||
)
|
||||
# → $IMPL transitions to blocked; run 1 closes with outcome='blocked'
|
||||
```
|
||||
|
||||
Now you (the human, or a separate reviewer profile) read the block reason, decide the fix direction is clear, and unblock from the dashboard's "Unblock" button — or from the CLI / slash command:
|
||||
|
||||
```bash
|
||||
hermes kanban unblock $IMPL
|
||||
# or from a chat: /kanban unblock $IMPL
|
||||
```
|
||||
|
||||
The dispatcher promotes `$IMPL` back to `ready` and, on the next tick, respawns the `backend-dev` worker. This second spawn is a **new run** on the same task:
|
||||
|
||||
```python
|
||||
# --- Engineer worker spawns on $IMPL (second attempt) ---
|
||||
# worker tool calls
|
||||
# --- Reviewer: request concrete changes ---
|
||||
kanban_show()
|
||||
# → worker_context now includes the run 1 block reason, so this worker knows
|
||||
# which two things to fix instead of re-reading the whole spec
|
||||
# (engineer adds zxcvbn check, makes reset tokens single-use, re-runs tests)
|
||||
kanban_complete(
|
||||
summary="added zxcvbn strength check, reset tokens are now single-use "
|
||||
"(stored + deleted on success)",
|
||||
# (inspect the handoff and candidate)
|
||||
kanban_request_changes(
|
||||
reason="Add password-strength validation and make reset tokens single-use."
|
||||
)
|
||||
# → the review run closes as outcome='changes_requested'; the card returns
|
||||
# to backend-dev in ready/todo without touching block-loop accounting
|
||||
|
||||
# --- Engineer: second implementation attempt ---
|
||||
kanban_show() # prior review evidence is in worker_context
|
||||
# (apply feedback and re-run tests)
|
||||
kanban_request_review(
|
||||
summary="added zxcvbn validation and single-use reset tokens",
|
||||
metadata={
|
||||
"changed_files": [
|
||||
"auth/reset.py",
|
||||
@@ -208,23 +188,21 @@ kanban_complete(
|
||||
"tests_run": 11,
|
||||
"review_iteration": 2,
|
||||
},
|
||||
reviewer="reviewer",
|
||||
)
|
||||
|
||||
# --- Reviewer: approve ---
|
||||
kanban_complete(summary="review passed; acceptance criteria verified")
|
||||
# → done
|
||||
```
|
||||
|
||||
Click the implementation task. The drawer shows **two attempts**:
|
||||
The task's run history now records `review_requested → changes_requested → review_requested → completed`. Each attempt has its own actor, summary, metadata, and outcome, so the second engineer sees exactly what the reviewer rejected and the final approval remains auditable. `kanban_block` is reserved for a real external escalation (missing access, a product decision, unavailable infrastructure), not normal review feedback.
|
||||
|
||||

|
||||
|
||||
- **Run 1** — `blocked` by `@backend-dev`. The review feedback sits right under the outcome: "password strength check missing, reset link isn't single-use (can be replayed within 30min)".
|
||||
- **Run 2** — `completed` by `@backend-dev`. Fresh summary, fresh metadata.
|
||||
|
||||
Each run is a row in `task_runs` with its own outcome, summary, and metadata. Retry history is not a conceptual afterthought layered on top of a "latest state" task — it's the primary representation. When a retrying worker opens the task, `build_worker_context` shows it the prior attempts, so the second-pass worker sees why the first pass was blocked and addresses those specific findings instead of re-running from scratch.
|
||||
|
||||
The reviewer picks up next. When they open `Review password reset PR`, they see:
|
||||
If you intentionally use the downstream-card model shown in the screenshot, the reviewer opens `Review password reset PR` after its implementation parent completes:
|
||||
|
||||

|
||||
|
||||
The parent link is the completed implementation. When the reviewer's worker spawns on `Review password reset PR` and calls `kanban_show()`, the returned `worker_context` includes the parent's most-recent-completed-run summary + metadata — so the reviewer reads "added zxcvbn strength check, reset tokens are now single-use" and has the list of changed files in hand before looking at a diff.
|
||||
The reviewer card's `worker_context` includes the completed implementation handoff. That is a separate card workflow; do not combine it with same-card `kanban_request_review` or you will duplicate the review lane.
|
||||
|
||||
## Story 4 — Circuit breaker and crash recovery
|
||||
|
||||
|
||||
+30
-53
@@ -148,56 +148,35 @@ dashboard 视图,按 `auth-project` 筛选:
|
||||
|
||||

|
||||
|
||||
三个阶段的链条一目了然:`Spec: password reset flow`(DONE,pm)、`Implement password reset flow`(DONE,backend-dev)、`Review password reset PR`(READY,reviewer)。每个任务底部都有绿色的父任务,以及作为依赖项的子任务。
|
||||
此截图使用**预创建下游审查卡**模型:实现卡有一个专用 reviewer 子卡。在该模型中,实现完成后工程师必须调用 `kanban_complete`,这样 reviewer 子卡才能离开 `todo`。不要为了请求审查而阻塞实现父卡。
|
||||
|
||||
最有趣的是实现任务,因为它经历了阻塞和重试。以下是完整的三 agent 协作流程,以每个 worker 模型发出的工具调用形式展示:
|
||||
如果同一张卡同时承载实现和审查,请改用一等 review lifecycle。完整的实现 → 审查 → 修改 → 再审流程如下:
|
||||
|
||||
```python
|
||||
# --- PM worker 在 $SPEC 上生成并编写验收标准 ---
|
||||
# worker tool calls
|
||||
# --- 工程师:第一次实现 ---
|
||||
kanban_show()
|
||||
kanban_complete(
|
||||
summary="spec approved; POST /forgot-password sends email, "
|
||||
"GET /reset/:token renders form, POST /reset applies new password",
|
||||
metadata={"acceptance": [
|
||||
"expired token returns 410",
|
||||
"reused last-3 password returns 400 with message",
|
||||
"successful reset invalidates all active sessions",
|
||||
]},
|
||||
# (编写代码、运行测试、准备候选版本)
|
||||
kanban_request_review(
|
||||
summary="implemented reset flow; candidate is ready for review",
|
||||
metadata={"changed_files": ["auth/reset.py"], "tests_run": 8},
|
||||
reviewer="reviewer",
|
||||
)
|
||||
# → $SPEC 完成;$IMPL 自动从 todo 提升为 ready
|
||||
# → 同一张卡进入 review;实现 run 以 outcome='review_requested' 关闭
|
||||
|
||||
# --- 工程师 worker 在 $IMPL 上生成(第一次尝试)---
|
||||
# worker tool calls
|
||||
kanban_show() # 在 worker_context 中读取 $SPEC 的 summary 和 acceptance metadata
|
||||
# (工程师编写代码,运行测试,开启 PR)
|
||||
# 审查者反馈到来——工程师认为问题有效并阻塞任务
|
||||
kanban_block(
|
||||
reason="Review: password strength check missing, reset link isn't "
|
||||
"single-use (can be replayed within 30min)",
|
||||
)
|
||||
# → $IMPL 转换为 blocked;run 1 以 outcome='blocked' 关闭
|
||||
```
|
||||
|
||||
现在你(人类,或单独的 reviewer profile)读取阻塞原因,判断修复方向明确,从 dashboard 的"Unblock"按钮解除阻塞——或通过 CLI/斜杠命令:
|
||||
|
||||
```bash
|
||||
hermes kanban unblock $IMPL
|
||||
# 或在聊天中:/kanban unblock $IMPL
|
||||
```
|
||||
|
||||
dispatcher 将 `$IMPL` 提升回 `ready`,并在下一次 tick 时重新生成 `backend-dev` worker。这第二次生成是同一任务上的**新 run**:
|
||||
|
||||
```python
|
||||
# --- 工程师 worker 在 $IMPL 上生成(第二次尝试)---
|
||||
# worker tool calls
|
||||
# --- Reviewer:请求修改 ---
|
||||
kanban_show()
|
||||
# → worker_context 现在包含 run 1 的阻塞原因,因此该 worker 知道
|
||||
# 需要修复哪两个问题,而无需重新阅读整个规格说明
|
||||
# (工程师添加 zxcvbn 检查,使重置令牌变为一次性,重新运行测试)
|
||||
kanban_complete(
|
||||
summary="added zxcvbn strength check, reset tokens are now single-use "
|
||||
"(stored + deleted on success)",
|
||||
# (检查 handoff 和候选版本)
|
||||
kanban_request_changes(
|
||||
reason="Add password-strength validation and make reset tokens single-use."
|
||||
)
|
||||
# → review run 以 outcome='changes_requested' 关闭;卡片返回 backend-dev
|
||||
# 的 ready/todo,且不会触碰 block-loop 计数
|
||||
|
||||
# --- 工程师:第二次实现 ---
|
||||
kanban_show() # worker_context 中包含之前的审查证据
|
||||
# (应用反馈并重新运行测试)
|
||||
kanban_request_review(
|
||||
summary="added zxcvbn validation and single-use reset tokens",
|
||||
metadata={
|
||||
"changed_files": [
|
||||
"auth/reset.py",
|
||||
@@ -207,23 +186,21 @@ kanban_complete(
|
||||
"tests_run": 11,
|
||||
"review_iteration": 2,
|
||||
},
|
||||
reviewer="reviewer",
|
||||
)
|
||||
|
||||
# --- Reviewer:批准 ---
|
||||
kanban_complete(summary="review passed; acceptance criteria verified")
|
||||
# → done
|
||||
```
|
||||
|
||||
点击实现任务,抽屉显示**两次尝试**:
|
||||
任务的 run 历史现在记录 `review_requested → changes_requested → review_requested → completed`。每次尝试都有独立的 actor、summary、metadata 和 outcome,因此第二次工程师运行能准确看到被拒绝的原因,最终批准也可审计。`kanban_block` 只用于真正的外部升级(缺少访问权限、产品决策、基础设施不可用),而不是普通审查反馈。
|
||||
|
||||

|
||||
|
||||
- **Run 1** — `@backend-dev` 标记为 `blocked`。审查反馈紧跟在结果下方:"password strength check missing, reset link isn't single-use (can be replayed within 30min)"。
|
||||
- **Run 2** — `@backend-dev` 标记为 `completed`。全新的 summary,全新的 metadata。
|
||||
|
||||
每个 run 在 `task_runs` 中都是独立的一行,有自己的 outcome、summary 和 metadata。重试历史不是叠加在"最新状态"任务之上的概念性附加物——它是主要的数据表示形式。当重试的 worker 打开任务时,`build_worker_context` 会向其展示之前的尝试,因此第二次 worker 能看到第一次被阻塞的原因,并针对性地解决那些具体问题,而不是从头重来。
|
||||
|
||||
审查者接下来认领任务。当他们打开 `Review password reset PR` 时,会看到:
|
||||
如果你有意使用截图中的下游卡模型,reviewer 会在实现父卡完成后打开 `Review password reset PR`:
|
||||
|
||||

|
||||
|
||||
父任务链接指向已完成的实现任务。当审查者的 worker 在 `Review password reset PR` 上生成并调用 `kanban_show()` 时,返回的 `worker_context` 包含父任务最近一次已完成 run 的 summary 和 metadata——因此审查者在查看 diff 之前就已读到"added zxcvbn strength check, reset tokens are now single-use",并掌握了变更文件列表。
|
||||
reviewer 卡的 `worker_context` 包含已完成实现的 handoff。这是独立卡工作流;不要再与同卡 `kanban_request_review` 混用,否则会重复创建审查通道。
|
||||
|
||||
## 场景四 — 熔断器与崩溃恢复
|
||||
|
||||
|
||||
Reference in New Issue
Block a user