Skip to content

feat(kanban): expose aggregate worker evidence - #83159

Open
moonweave wants to merge 1 commit into
NousResearch:mainfrom
moonweave:codex/worker-evidence-contract-20260810-reviewed
Open

feat(kanban): expose aggregate worker evidence#83159
moonweave wants to merge 1 commit into
NousResearch:mainfrom
moonweave:codex/worker-evidence-contract-20260810-reviewed

Conversation

@moonweave

Copy link
Copy Markdown

What does this PR do?

Adds an additive, aggregate-only worker-evidence projection to hermes kanban stats --json so consumers can distinguish a recorded running row from a host-locally verified live worker.

The projection reports:

  • per-assignee and total running/live/stale/unverified row counts;
  • unassigned running totals;
  • the latest aggregate heartbeat timestamp per assignee.

A live worker requires both a host-local live PID and a recent heartbeat. Remote-host rows and insufficient evidence remain unverified. A stale heartbeat or a dead host-local PID is reported stale.

All aggregate queries are read from one SQLite snapshot so by_status.running, by_assignee[*].running, and the new activity totals cannot describe different moments during concurrent worker updates.

No task ID, title, body, result, comment, claim lock, worker PID, hostname, path, or email is added to the JSON contract.

Related Issue

No linked issue.

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • Add activity_by_assignee and activity_totals to board stats.
  • Classify live, stale, and unverified evidence without promoting running alone to live.
  • Preserve existing by_status, by_assignee, age, and JSON behavior.
  • Hold one SQLite read snapshot across all aggregate queries.
  • Add behavioral coverage for live/dead/stale-heartbeat/remote/unassigned rows, privacy, legacy-count invariants, latest-heartbeat selection, and a concurrent writer race.

How to Test

  1. Run:
    scripts/run_tests.sh tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_cli.py
  2. Run:
    ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py
  3. Inspect hermes kanban stats --json and verify the three count invariants documented above.

Local result: 57 passed, 0 failed, 2 skipped; Ruff passed.

Checklist

Code

  • I've read the Contributing Guide.
  • My commit message follows Conventional Commits.
  • I searched existing PRs for the exact aggregate contract keys; no matches.
  • My PR contains only the two related core/test files.
  • Full repository test suite (focused Kanban DB/CLI suite passed: 57/57 executed tests)
  • I've added behavioral tests for the change.
  • Tested on macOS.

Documentation & Housekeeping

  • Relevant documentation: N/A; this is an additive JSON response contract covered in code docstrings/tests.
  • cli-config.yaml.example: N/A; no config keys.
  • CONTRIBUTING.md/AGENTS.md: N/A; no workflow or architecture change.
  • Cross-platform impact considered; liveness uses existing cross-platform _pid_alive.
  • Tool descriptions/schemas: N/A; no model tool schema change.

Runtime compatibility evidence

Against the reviewed commit, the dashboard adapter reported connected freshness, 20/20 worker-evidence contract profiles, 0 fallback profiles, true evidence invariants, and no prohibited key paths. The clean upstream base reported 0/20 contract profiles and 20/20 safe legacy fallback profiles. Current runtime had zero running rows, so this evidence does not claim any worker was active.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Aug 10, 2026

Copy link
Copy Markdown
Author

The PR workflows are currently blocked with conclusion action_required, consistent with a first-time fork contribution. Could a maintainer approve and run the workflows? The focused Kanban DB/CLI suite is green (57 passed, 0 failed, 2 skipped), and the full-suite checklist remains intentionally unchecked until upstream CI reports.

@enzo-adami

Copy link
Copy Markdown
Contributor

Independent validation on current origin/main (165c889e5b4277b56dadd42949a4112c1e6175a6): the PR patch from 58b0944553 applies cleanly (including the kanban_db.py overlap), git diff --check passes, and the full targeted file passes through the repository runner:

scripts/run_tests.sh tests/hermes_cli/test_kanban_core_functionality.py -q
25 passed, 0 failed (1 skipped)

I also reviewed the aggregate contract: remote-host rows remain unverified, local liveness requires both a live PID and a fresh heartbeat, task IDs/content/claim locks/PIDs are excluded, and all aggregate queries are held in one SQLite read snapshot. I did not find a counterexample in that scope.

Compatibility note for the worker-lifecycle work in #86609/#86610: their kanban_db.py patch hunks apply after this PR's patch without a content conflict. #86609 currently has a separate env_loader.py rebase conflict against today's main, unrelated to this PR.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(kanban): expose aggregate worker evidence

  1. hermes_cli/kanban_db.py:53_pid_alive(int(pid)): if worker_pid ever holds non-numeric garbage (older rows, manual edits, recovered checkpoints), int(pid) raises ValueError and board_stats() crashes for the whole board. Guarding the conversion (try/except or an isdigit() check) would make the aggregation robust to dirty rows.

  2. hermes_cli/kanban_db.py:24 — host-locality is decided by claim_lock.startswith(f"{_claimer_id().split(':', 1)[0]}:"). In container/VM setups where hosts share a hostname, a remote claim could be misclassified as host-local, flipping its evidence from "unverified" to "live"/"stale" incorrectly. It's a reasonable heuristic — a fallback that also requires the claimer id to match (not just the host prefix) or a comment documenting the assumption would harden it.

  3. hermes_cli/kanban_db.py:111-126 — wrapping the aggregation in BEGIN/rollback gives a consistent read snapshot when the caller isn't already in a transaction; when it is (owns_snapshot=False) the halves read the caller's transaction snapshot, so consistency holds either way. Good.

The no-leak test (asserting task ids/titles/bodies never appear in the serialized payload) is exactly the right invariant for an aggregate endpoint that routers consume.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants