Skip to content

fix(kanban): teach workers valid foreground terminal calls - #125303

Closed
brownser wants to merge 1 commit into
NousResearch:mainfrom
brownser:fix/kanban-foreground-terminal-20260927
Closed

brownser wants to merge 1 commit into
NousResearch:mainfrom
brownser:fix/kanban-foreground-terminal-20260927

Conversation

@brownser

Copy link
Copy Markdown

Observed Banger Ledger coder and raven worker sessions repeatedly send heartbeat=60 with foreground terminal calls; terminal correctly rejects this background-only argument, and runs timed out without accepted artifacts. Add worker-only guidance to omit notify/heartbeat on foreground calls, correct invalid arguments once, and never reroute a security denial. Regression covers prompt guidance and handler contract. Tests: 55 passed, 2 skipped (prompt builder + process heartbeat). No runtime activation or review verdict claimed. Rollback: revert 162e464.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management tool/terminal Terminal execution and process management sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) labels Sep 27, 2026

kvnloo commented Sep 27, 2026

Copy link
Copy Markdown

Independent exact-head check on 162e464893aafac3adaa8cc3e41e2b105f77529b: I mirrored this patch in kvnloo#198 and ran the kanban worker prompt regression.

  • focused patched case: 1 passed
  • full test_prompt_builder.py: 55 passed
  • negative control: 1 failed
  • restored: focused case + full file green

This confirms the worker guidance now distinguishes foreground terminal calls from background=true calls that may use notify / heartbeat.

@teknium1

teknium1 commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Heads-up: #133793 lands this class at the dispatcher instead — a foreground terminal call that carries a positive heartbeat runs once with the heartbeat dropped (cherry-pick of #119201 with authorship kept, plus the test/schema follow-up), while a foreground notify=true stays refused. Live repro in that PR body uses the exact argument shapes recorded from 173 production refusals this week. If it merges, this PR becomes unnecessary and will be closed as superseded with credit; leaving it open until then.

@teknium1 teknium1 added the needs-decision Awaiting maintainer decision before any implementation label Oct 6, 2026
@teknium1

teknium1 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #133793 (landed as cb6ffe6): a foreground terminal call that carries heartbeat now runs instead of being refused, so kanban workers no longer need a prompt-side workaround. Thanks @brownser for tracking this down.

@teknium1 teknium1 closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants