chore: trim observer test suites to core coverage and condense hooks docs
Per review: keep the load-bearing tests (fire+payload per hook, the lock-probe contract test, misbehaving-subscriber isolation, no-subscriber short-circuit, mutation-boundary coverage) and drop the auxiliary variants; compress the hooks.md additions to a single catalog-row set plus a compact bullet section.
This commit is contained in:
Teknium
parent
5e10351683
commit
6ce0231478
|
|
@ -44,27 +44,6 @@ def captured_ticks(monkeypatch):
|
|||
finally:
|
||||
mgr._hooks = saved
|
||||
|
||||
|
||||
def test_dispatch_tick_hook_is_registered_as_valid():
|
||||
assert "on_kanban_dispatch_tick" in VALID_HOOKS
|
||||
|
||||
|
||||
def test_idle_tick_fires_hook_with_outcome_idle(kanban_home, captured_ticks):
|
||||
"""An empty board produces one hook fire with outcome='idle'."""
|
||||
conn = kb.connect()
|
||||
try:
|
||||
kb.dispatch_once(conn, spawn_fn=lambda *a, **k: 12345)
|
||||
finally:
|
||||
conn.close()
|
||||
assert len(captured_ticks) == 1
|
||||
kw = captured_ticks[0]
|
||||
assert kw["outcome"] == "idle"
|
||||
assert kw["dry_run"] is False
|
||||
assert isinstance(kw["result"], kb.DispatchResult)
|
||||
assert "board" in kw
|
||||
assert "profile_name" in kw
|
||||
|
||||
|
||||
def test_active_tick_fires_hook_with_outcome_ok(
|
||||
kanban_home, all_assignees_spawnable, captured_ticks,
|
||||
):
|
||||
|
|
@ -82,35 +61,6 @@ def test_active_tick_fires_hook_with_outcome_ok(
|
|||
result = ok_events[-1]["result"]
|
||||
assert any(row[0] == tid for row in result.spawned)
|
||||
|
||||
|
||||
def test_dry_run_tick_carries_dry_run_flag(kanban_home, captured_ticks):
|
||||
"""dry_run=True is propagated to the hook payload."""
|
||||
conn = kb.connect()
|
||||
try:
|
||||
kb.dispatch_once(conn, dry_run=True, spawn_fn=lambda *a, **k: None)
|
||||
finally:
|
||||
conn.close()
|
||||
assert captured_ticks
|
||||
assert all(kw["dry_run"] is True for kw in captured_ticks)
|
||||
|
||||
|
||||
def test_contended_tick_fires_with_outcome_skipped_locked(
|
||||
kanban_home, captured_ticks,
|
||||
):
|
||||
"""A losing dispatcher still reports its (empty) tick to observers."""
|
||||
db_path = kb.kanban_db_path()
|
||||
conn = kb.connect()
|
||||
try:
|
||||
with kb._dispatch_tick_lock(db_path) as held:
|
||||
assert held is True
|
||||
result = kb.dispatch_once(conn, spawn_fn=lambda *a, **k: 1)
|
||||
assert result.skipped_locked is True
|
||||
finally:
|
||||
conn.close()
|
||||
assert len(captured_ticks) == 1
|
||||
assert captured_ticks[0]["outcome"] == "skipped_locked"
|
||||
|
||||
|
||||
def test_tick_hook_fires_after_dispatch_lock_released(kanban_home):
|
||||
"""The #56066 sweeper finding, as a contract: subscribers run OUTSIDE
|
||||
the single-writer critical section. From the callback, acquiring the
|
||||
|
|
|
|||
|
|
@ -40,11 +40,6 @@ def captured_updates(monkeypatch):
|
|||
finally:
|
||||
mgr._hooks = saved
|
||||
|
||||
|
||||
def test_task_updated_hook_is_registered_as_valid():
|
||||
assert "on_kanban_task_updated" in VALID_HOOKS
|
||||
|
||||
|
||||
def test_assign_fires_updated_with_changed_fields(kanban_home, captured_updates):
|
||||
"""assign_task fires the observer post-commit with the changed field."""
|
||||
assignee_at_fire_time: list = []
|
||||
|
|
@ -82,65 +77,6 @@ def test_assign_fires_updated_with_changed_fields(kanban_home, captured_updates)
|
|||
assert "run_id" in kw
|
||||
assert assignee_at_fire_time == ["bob"]
|
||||
|
||||
|
||||
def test_set_model_override_fires_with_changed_fields(
|
||||
kanban_home, captured_updates,
|
||||
):
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="alice")
|
||||
captured_updates.clear()
|
||||
assert kb.set_model_override(
|
||||
conn, tid, "some-model", provider="some-provider",
|
||||
) is True
|
||||
finally:
|
||||
conn.close()
|
||||
assert len(captured_updates) == 1
|
||||
kw = captured_updates[0]
|
||||
assert kw["task_id"] == tid
|
||||
assert kw["changed_fields"] == ["model_override", "provider_override"]
|
||||
|
||||
|
||||
def test_set_reasoning_effort_fires_with_changed_fields(
|
||||
kanban_home, captured_updates,
|
||||
):
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="alice")
|
||||
captured_updates.clear()
|
||||
assert kb.set_reasoning_effort(conn, tid, "high") is True
|
||||
finally:
|
||||
conn.close()
|
||||
assert len(captured_updates) == 1
|
||||
kw = captured_updates[0]
|
||||
assert kw["task_id"] == tid
|
||||
assert kw["changed_fields"] == ["reasoning_effort"]
|
||||
|
||||
|
||||
def test_assign_missing_task_does_not_fire(kanban_home, captured_updates):
|
||||
conn = kb.connect()
|
||||
try:
|
||||
assert kb.assign_task(conn, "kb-nope", "bob") is False
|
||||
finally:
|
||||
conn.close()
|
||||
assert captured_updates == []
|
||||
|
||||
|
||||
def test_assign_running_task_raises_and_does_not_fire(
|
||||
kanban_home, captured_updates,
|
||||
):
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="alice")
|
||||
kb.claim_task(conn, tid)
|
||||
captured_updates.clear()
|
||||
with pytest.raises(RuntimeError):
|
||||
kb.assign_task(conn, tid, "bob")
|
||||
finally:
|
||||
conn.close()
|
||||
assert captured_updates == []
|
||||
|
||||
|
||||
def test_raising_callback_does_not_break_assign(kanban_home):
|
||||
mgr = get_plugin_manager()
|
||||
saved = {k: list(v) for k, v in mgr._hooks.items()}
|
||||
|
|
|
|||
|
|
@ -55,12 +55,6 @@ def captured_hooks(monkeypatch):
|
|||
finally:
|
||||
mgr._hooks = saved
|
||||
|
||||
|
||||
def test_worker_lifecycle_hooks_are_registered_as_valid():
|
||||
for hook in WORKER_HOOKS:
|
||||
assert hook in VALID_HOOKS, hook
|
||||
|
||||
|
||||
def test_dispatch_spawn_fires_worker_spawned(
|
||||
kanban_home, all_assignees_spawnable, captured_hooks,
|
||||
):
|
||||
|
|
@ -102,24 +96,6 @@ def test_dispatch_spawn_fires_worker_spawned(
|
|||
assert "board" in kw
|
||||
assert pid_at_fire_time == [4242]
|
||||
|
||||
|
||||
def test_spawn_without_pid_fires_with_worker_pid_none(
|
||||
kanban_home, all_assignees_spawnable, captured_hooks,
|
||||
):
|
||||
"""A spawn_fn that reports no PID still fires, with worker_pid=None."""
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="alice")
|
||||
kb.dispatch_once(conn, spawn_fn=lambda *a, **k: None)
|
||||
finally:
|
||||
conn.close()
|
||||
fired = [e for e in captured_hooks if e[0] == "on_kanban_worker_spawned"]
|
||||
assert len(fired) == 1
|
||||
kw = fired[0][1]
|
||||
assert kw["task_id"] == tid
|
||||
assert kw["worker_pid"] is None
|
||||
|
||||
|
||||
def test_crash_reclaim_fires_worker_exited(kanban_home, captured_hooks, monkeypatch):
|
||||
"""A dead-PID reclaim fires the exit observer with the exit facts."""
|
||||
conn = kb.connect()
|
||||
|
|
@ -146,32 +122,6 @@ def test_crash_reclaim_fires_worker_exited(kanban_home, captured_hooks, monkeypa
|
|||
assert "profile_name" in kw
|
||||
assert "board" in kw
|
||||
|
||||
|
||||
def test_rate_limited_exit_fires_worker_exited_with_outcome(
|
||||
kanban_home, captured_hooks, monkeypatch,
|
||||
):
|
||||
"""The EX_TEMPFAIL sentinel classifies as a rate_limited exit."""
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="worker")
|
||||
kb.claim_task(conn, tid)
|
||||
kb._set_worker_pid(conn, tid, 24680)
|
||||
# Simulate the reap loop having recorded the sentinel exit status.
|
||||
kb._record_worker_exit(24680, kb.KANBAN_RATE_LIMIT_EXIT_CODE << 8)
|
||||
monkeypatch.setattr(kb, "_pid_alive", lambda pid: False)
|
||||
assert kb.detect_crashed_workers(conn) == []
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
fired = [e for e in captured_hooks if e[0] == "on_kanban_worker_exited"]
|
||||
assert len(fired) == 1
|
||||
kw = fired[0][1]
|
||||
assert kw["task_id"] == tid
|
||||
assert kw["exit_kind"] == "rate_limited"
|
||||
assert kw["exit_code"] == kb.KANBAN_RATE_LIMIT_EXIT_CODE
|
||||
assert kw["outcome"] == "rate_limited"
|
||||
|
||||
|
||||
def test_stale_claim_reclaim_fires_hook(kanban_home, captured_hooks):
|
||||
"""A TTL-expired reclaim fires the stale-claim observer post-commit."""
|
||||
conn = kb.connect()
|
||||
|
|
@ -199,29 +149,6 @@ def test_stale_claim_reclaim_fires_hook(kanban_home, captured_hooks):
|
|||
assert "profile_name" in kw
|
||||
assert "board" in kw
|
||||
|
||||
|
||||
def test_claim_extension_does_not_fire_stale_claim(
|
||||
kanban_home, captured_hooks, monkeypatch,
|
||||
):
|
||||
"""A live-PID claim extension is not a reclaim and must not fire."""
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(conn, title="t", assignee="worker")
|
||||
kb.claim_task(conn, tid)
|
||||
kb._set_worker_pid(conn, tid, 13579)
|
||||
monkeypatch.setattr(kb, "_pid_alive", lambda pid: True)
|
||||
conn.execute(
|
||||
"UPDATE tasks SET claim_expires = ? WHERE id = ?",
|
||||
(int(time.time()) - 100, tid),
|
||||
)
|
||||
conn.commit()
|
||||
assert kb.release_stale_claims(conn) == 0
|
||||
finally:
|
||||
conn.close()
|
||||
fired = [e for e in captured_hooks if e[0] == "on_kanban_worker_stale_claim"]
|
||||
assert fired == []
|
||||
|
||||
|
||||
def test_raising_callbacks_never_break_worker_lifecycle(
|
||||
kanban_home, all_assignees_spawnable, monkeypatch,
|
||||
):
|
||||
|
|
|
|||
|
|
@ -85,31 +85,6 @@ def test_patch_priority_fires_task_updated(client, captured_updates):
|
|||
assert kw["changed_fields"] == ["priority"]
|
||||
assert kw["board"]
|
||||
|
||||
|
||||
def test_patch_title_and_body_fires_task_updated(client, captured_updates):
|
||||
tid = _make_task()
|
||||
captured_updates.clear()
|
||||
r = client.patch(
|
||||
f"/api/plugins/kanban/tasks/{tid}",
|
||||
json={"title": "new title", "body": "new body"},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
assert len(captured_updates) == 1
|
||||
kw = captured_updates[0]
|
||||
assert kw["task_id"] == tid
|
||||
assert kw["changed_fields"] == ["title", "body"]
|
||||
|
||||
|
||||
def test_patch_empty_title_rejected_does_not_fire(client, captured_updates):
|
||||
tid = _make_task()
|
||||
captured_updates.clear()
|
||||
r = client.patch(
|
||||
f"/api/plugins/kanban/tasks/{tid}", json={"title": " "},
|
||||
)
|
||||
assert r.status_code == 400
|
||||
assert captured_updates == []
|
||||
|
||||
|
||||
def test_bulk_priority_fires_task_updated_per_task(client, captured_updates):
|
||||
tid1 = _make_task("a")
|
||||
tid2 = _make_task("b")
|
||||
|
|
@ -123,17 +98,3 @@ def test_bulk_priority_fires_task_updated_per_task(client, captured_updates):
|
|||
fired = {kw["task_id"]: kw for kw in captured_updates}
|
||||
assert set(fired) == {tid1, tid2}
|
||||
assert all(kw["changed_fields"] == ["priority"] for kw in fired.values())
|
||||
|
||||
|
||||
def test_patch_assignee_fires_once_via_assign_task(client, captured_updates):
|
||||
"""The dashboard assignee path goes through kanban_db.assign_task; the
|
||||
dashboard layer must not double-fire it."""
|
||||
tid = _make_task()
|
||||
captured_updates.clear()
|
||||
r = client.patch(
|
||||
f"/api/plugins/kanban/tasks/{tid}", json={"assignee": "bob"},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
fired = [kw for kw in captured_updates if kw["task_id"] == tid]
|
||||
assert len(fired) == 1
|
||||
assert fired[0]["changed_fields"] == ["assignee"]
|
||||
|
|
|
|||
|
|
@ -468,11 +468,11 @@ Payload fields below are the exact event-specific fields supplied by each call s
|
|||
| `kanban_task_claimed` | Observer | After claim commit, in dispatcher process before worker spawn; return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id` | Board/task/profile/assignee identifiers. |
|
||||
| `kanban_task_completed` | Observer | After completion and cleanup, usually in worker process; return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `summary` | Summary may contain project/user content. |
|
||||
| `kanban_task_blocked` | Observer | After a blocked transition; the dependency-wait path fires before its transaction exits. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `reason` | Reason may contain project/user content. |
|
||||
| `on_kanban_worker_spawned` | Observer | In the dispatcher process, after `spawn_fn` returns and the worker PID (when one was reported) is durably persisted; runs inside the board dispatch lock like `kanban_task_claimed`, so callbacks must stay fast. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `workspace_path` | `workspace_path` is a filesystem path and may reveal project layout or usernames. |
|
||||
| `on_kanban_worker_exited` | Observer | Tick-derived in the dispatcher process: fires when `detect_crashed_workers` reclaims a dead-PID running task, after every reclaim and breaker-accounting transaction has committed; exit latency is bounded by the dispatcher tick interval. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `exit_kind`, `exit_code`, `outcome`, `retry_status` | Identifiers and exit metadata only. |
|
||||
| `on_kanban_worker_stale_claim` | Observer | In the dispatcher process when `release_stale_claims` reclaims a TTL-expired claim, after the reclaim transaction commits; live-PID claim extensions and deferred reclaims do not fire. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `heartbeat_stale`, `retry_status` | Identifiers and claim metadata only. |
|
||||
| `on_kanban_task_updated` | Observer | After a committed task-row field mutation outside the claim/complete/block lifecycle: `assign_task`, `set_model_override`, `set_reasoning_effort`, plus the dashboard plugin API's direct-SQL priority/title/body editors (single and bulk) via `kanban_db.notify_task_updated`. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `changed_fields` | `changed_fields` carries field names only, never values; the named title/body values in the board DB may contain user/project content. |
|
||||
| `on_kanban_dispatch_tick` | Observer | Once per dispatcher tick in `dispatch_once`, strictly after the board's single-writer dispatch lock is released (never inside the writer critical section); also fires for idle and lock-contended ticks. Return ignored. | `board`, `profile_name`, `dry_run`, `outcome`, `result` | `result` is the tick's `DispatchResult` and carries task ids, assignees, and workspace paths. |
|
||||
| `on_kanban_worker_spawned` | Observer | After `spawn_fn` returns and the worker PID is persisted; runs inside the dispatch lock, keep callbacks fast. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `workspace_path` | `workspace_path` is a filesystem path and may reveal project layout or usernames. |
|
||||
| `on_kanban_worker_exited` | Observer | Tick-derived: after `detect_crashed_workers` reclaims a dead-PID task and the reclaim commits. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `exit_kind`, `exit_code`, `outcome`, `retry_status` | Identifiers and exit metadata only. |
|
||||
| `on_kanban_worker_stale_claim` | Observer | After a TTL-expired claim is reclaimed; live-PID extensions don't fire. Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `worker_pid`, `heartbeat_stale`, `retry_status` | Identifiers and claim metadata only. |
|
||||
| `on_kanban_task_updated` | Observer | After a committed task-field write outside the claim/complete/block lifecycle (assign, overrides, dashboard editors). Return ignored. | `task_id`, `profile_name`, `board`, `assignee`, `run_id`, `changed_fields` | `changed_fields` carries field names only, never values; the named title/body values in the board DB may contain user/project content. |
|
||||
| `on_kanban_dispatch_tick` | Observer | Once per dispatcher tick, strictly after the dispatch lock is released; idle and contended ticks fire too. Return ignored. | `board`, `profile_name`, `dry_run`, `outcome`, `result` | `result` is the tick's `DispatchResult` and carries task ids, assignees, and workspace paths. |
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -1528,27 +1528,13 @@ All three kanban hooks are observer-only and carry `task_id`, `profile_name`, `b
|
|||
|
||||
### Kanban worker-lifecycle, task-mutation, and dispatch observers
|
||||
|
||||
These observers extend the kanban lifecycle family with the worker and mutation events accepted from RFC #58548 (batch disposition in #64231). All of them are observer-only, fire only after the relevant write transaction has committed, and short-circuit on `has_hook` — with no subscriber registered, no payload is built and dispatch behavior is unchanged. The task-scoped hooks carry the same common fields as the shipped kanban hooks: `task_id`, `profile_name`, `board`, `assignee`, and `run_id`.
|
||||
Five additional observers (RFC #58548) extend the kanban family. All are observer-only, fire after the relevant transaction commits, and short-circuit on `has_hook` — with no subscriber, dispatch behavior is unchanged. Task-scoped hooks carry the same common fields as the hooks above.
|
||||
|
||||
#### `on_kanban_worker_spawned`
|
||||
|
||||
Fires in the dispatcher process after `spawn_fn` returns and the worker PID (when one was reported) has been durably persisted. Adds `worker_pid` (may be `None` when the spawn function reports no PID) and `workspace_path`. Like `kanban_task_claimed`, it runs inside the board's dispatch lock, so callbacks must stay fast.
|
||||
|
||||
#### `on_kanban_worker_exited`
|
||||
|
||||
Worker exits are tick-derived: this fires when `detect_crashed_workers` reclaims a running task whose worker PID is no longer alive, after every reclaim and breaker-accounting transaction has committed. Exit visibility latency is therefore bounded by the dispatcher tick interval. Adds `worker_pid`, `exit_kind` (`clean_exit` | `rate_limited` | `nonzero_exit` | `signaled` | `unknown`), `exit_code`, `outcome` (`crashed` | `rate_limited`), and `retry_status` (the phase the task was released back to).
|
||||
|
||||
#### `on_kanban_worker_stale_claim`
|
||||
|
||||
Fires in the dispatcher process when `release_stale_claims` reclaims a TTL-expired claim, after the reclaim transaction commits. Live-PID claim extensions and deferred reclaims do not fire. Adds `worker_pid`, `heartbeat_stale`, and `retry_status`.
|
||||
|
||||
#### `on_kanban_task_updated`
|
||||
|
||||
The task-mutation boundary observer. Fires after a committed task-row field write outside the claim/complete/block lifecycle: `kanban_db.assign_task`, `set_model_override`, and `set_reasoning_effort`, plus the dashboard plugin API's direct-SQL priority/title/body editors (single-task and bulk), which report through `kanban_db.notify_task_updated`. Adds `changed_fields`, a list of field names only — new values are never carried in the payload; fetch the task if you need them. Status transitions stay in the lifecycle hook family, and dispatcher bookkeeping fields (worker PID, workspace path, claim columns) are deliberately excluded as noise.
|
||||
|
||||
#### `on_kanban_dispatch_tick`
|
||||
|
||||
Fires once per dispatcher tick in `dispatch_once`, strictly after the board's single-writer dispatch lock has been released — never inside the writer critical section — so a slow subscriber cannot stall a sibling dispatcher. Idle and lock-contended ticks fire too. It is not task-scoped; its payload is `board`, `profile_name`, `dry_run`, `outcome` (`ok` | `skipped_locked` | `idle`), and `result` (the tick's `DispatchResult`).
|
||||
- **`on_kanban_worker_spawned`** — after `spawn_fn` returns and the worker PID is persisted. Adds `worker_pid` (may be `None`) and `workspace_path`. Runs inside the dispatch lock; keep callbacks fast.
|
||||
- **`on_kanban_worker_exited`** — tick-derived, when `detect_crashed_workers` reclaims a dead-PID task. Adds `worker_pid`, `exit_kind`, `exit_code`, `outcome`, `retry_status`.
|
||||
- **`on_kanban_worker_stale_claim`** — when a TTL-expired claim is reclaimed; live-PID extensions don't fire. Adds `worker_pid`, `heartbeat_stale`, `retry_status`.
|
||||
- **`on_kanban_task_updated`** — after a committed task-field write outside the claim/complete/block lifecycle (`assign_task`, model/reasoning overrides, dashboard editors). Adds `changed_fields` — field names only, never values.
|
||||
- **`on_kanban_dispatch_tick`** — once per dispatcher tick, strictly after the dispatch lock is released, including idle and lock-contended ticks. Payload: `board`, `profile_name`, `dry_run`, `outcome`, `result`.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue