Skip to content

fix(kanban): persist summary-only completion results - #70978

Open
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/kanban-completion-delivery-t_343e914f
Open

fix(kanban): persist summary-only completion results#70978
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/kanban-completion-delivery-t_343e914f

Conversation

@trac3r00

@trac3r00 trac3r00 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

[Bob]

Summary

  • persist summary into tasks.result when callers omit result
  • preserve an explicit result when both fields are supplied
  • cover claimed worker runs and manual completion's synthesized-run path

Root cause

complete_task() already copied result into task_runs.summary when callers omitted summary, but not the reverse. Dispatcher workers commonly complete through the run protocol with only a rich summary, so the run row closed successfully while tasks.result remained NULL. Dashboards and delivery paths that read the task row then had no durable completion text.

Duplicate search

The notification-inheritance half of the original local fix was removed because open PR #57365 already covers parent-to-child subscription inheritance more comprehensively across create_task, link_tasks, and triage decomposition. This PR keeps only the distinct summary-to-result persistence bug. Merged PR #20195 addresses dashboard-entered summaries and recovery editing, not summary-only worker completions.

RED → GREEN

On origin/main, the regression cases produced:

2 failed, 2 passed

The failures were the claimed-run and synthesized-run paths leaving task.result as None.

After the fix:

417 passed, 1 skipped

Verification

HERMES_DELEGATED_CHILD_CONTEXT= python -m pytest \
  tests/hermes_cli/test_kanban_notify.py \
  tests/hermes_cli/test_kanban_core_functionality.py \
  tests/hermes_cli/test_kanban_db.py -q

417 passed, 1 skipped
python -m ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
All checks passed!

python -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
git diff HEAD^ HEAD --check

Independent fail-closed review passed with no security concerns or logic errors.

[bob]

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Jul 24, 2026
Update dashboard integration expectations for the durable result contract.

[bob]
@trac3r00
trac3r00 force-pushed the fix/kanban-completion-delivery-t_343e914f branch from dacac30 to d889027 Compare July 24, 2026 20:54

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the summary-only completion case and adding both claimed-run and synthesized-run coverage.

Problems

  • Current main deliberately separates the worker handoff from the legacy task-row result: summary is the run-level handoff, while result is an explicit legacy task-row log line (tools/kanban_tools.py:1561-1585; website/docs/user-guide/features/kanban.md:913-919). The new fallback changes that contract.
  • The claimed display/delivery gap is already handled on current main: dashboard API attaches latest_summary (plugins/kanban/dashboard/plugin_api.py:456-467, :543-552), CLI shows it when result is empty (hermes_cli/kanban.py:1750-1760), and notifier delivery prefers the event summary (gateway/kanban_watchers.py:407-424). Commit 3f972974133659a366f5d63b01423a4709c507b3 added this path intentionally.

Suggested changes

  • Please identify a current consumer that cannot use the run/event summary and add an end-to-end regression for that path. If the intended change is to make the two fields aliases, re-scope it as a contract change including tool-schema and documentation updates.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
# passed summary (the common run-protocol shape). Without this the task
# closes with result=NULL and the summary is only reachable via the run
# row — dashboard/notifier paths that read tasks.result show nothing.
if result is None and summary is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

summary and result are intentionally distinct on current main: the tool schema directs workers to use the run-level summary, while result is a legacy task-row field. Dashboard, CLI, and notifier consumers already surface the summary. Please retain that distinction unless this is explicitly re-scoped as a cross-surface contract change.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants