Skip to content

Strip tools from webhook reply-gen + fair preempt yield + more color#517

Merged
FidoCanCode merged 2 commits into
mainfrom
fix-webhook-reply-tools-and-more-color
Apr 15, 2026
Merged

Strip tools from webhook reply-gen + fair preempt yield + more color#517
FidoCanCode merged 2 commits into
mainfrom
fix-webhook-reply-tools-and-more-color

Conversation

@FidoCanCode
Copy link
Copy Markdown
Owner

Summary

Three fixes for the webhook responsiveness regression on PR #515:

  1. Reply-gen disallows tools. Observed 2026-04-14: rhencke's review comment on PR Tastefully colorize kennel status output (closes #511) #515 arrived at 23:36:15, reply posted at 23:41:32 — 4m 44s. The JSONL showed Opus had fired Bash/Read/Edit calls on test_status.py inside the reply-gen turn, treating the review comment as a directive. reply_system_prompt now strictly forbids tool use and explains the worker will do the actual work later from the task queue.

  2. Fair preempt yield via _preempt_pending event. Python's RLock isn't FIFO-fair under contention. A worker thread that released on _cancel could race back in and re-acquire before the waiting webhook thread got scheduled; iter_events._cancel.clear() at the next turn then wiped the webhook's signal, and the webhook waited a full worker turn to actually run. session.prompt() now sets _preempt_pending right after _cancel.set(), __enter__ clears it on acquire, and the worker's retry loop calls session.wait_for_pending_preempt() between release and re-acquire.

  3. More ip-color style coverage. Repo name BOLD + state word GREEN/DIM separately, Issue/PR/Worker labels BOLD, tree glyphs and claude pid suffix DIM, kennel: UP in GREEN and DOWN in RED_BOLD.

Logging added: session.prompt logs preempt request; wait_for_pending_preempt logs cede + acquire-latency or timeout warning.

Test plan

  • 1823 tests, 100% coverage, ruff clean, pyright clean
  • After merge: webhook review replies post in seconds, not minutes
  • After merge: log shows session: preempter acquired lock after <short>s yield on webhook preempts

Three fixes for the webhook responsiveness regression observed on PR #515:

1. Reply-gen system prompt now forbids tool use.  Opus was treating
   review comments as directives and firing Bash/Read/Edit calls to
   actually make the change — a 5s reply turned into a 4m 44s session
   turn that held the lock and starved the worker.  `reply_system_prompt`
   in prompts.py now spells out 'no Bash, no Read, no Edit, no Write,
   no Grep, no Glob, no Task sub-agents, no WebFetch, no plan mode, no
   file modifications of any kind' and explains that the task queue will
   do the actual work later.

2. Fair preempt yield via `_preempt_pending` event.  Python's RLock
   isn't FIFO-fair under contention, so a worker thread that released
   the lock on cancellation could race back in and re-acquire before
   the waiting webhook thread got scheduled — at which point
   iter_events' `_cancel.clear()` at turn start wiped the webhook's
   signal and the webhook waited for a full worker turn.  prompt() now
   sets `_preempt_pending` right after _cancel.set(), __enter__ clears
   it on acquire, and the worker's `_run_session_turn` retry calls
   `session.wait_for_pending_preempt()` between release and re-acquire
   so the preempter actually gets the lock first.

3. More ip-color style coverage in kennel/status.py — repo name BOLD +
   state word GREEN/DIM separately, Issue/PR/Worker labels BOLD, tree
   glyphs DIM, claude pid suffix DIM, kennel: UP in GREEN and DOWN in
   RED_BOLD.

Logging added on the preempt path: `session.prompt` logs preempt
request, `wait_for_pending_preempt` logs cede + acquire-latency or
timeout-warning.
@FidoCanCode FidoCanCode requested a review from rhencke April 14, 2026 23:57
Adds `ClaudeSession._log_event` that renders each stream-json event as a
human-readable INFO line, invoked both inside iter_events and inside
_drain_to_boundary (so a cancelled turn's tail is visible rather than
silently discarded).  Also flips the default --log-level from INFO to
DEBUG so new debug-level trace points light up without operator action.

Event shapes rendered:
- assistant text → 'claude> <text>'
- assistant tool_use → 'claude tool: <name> <command/file_path/first-arg>'
- user tool_result → 'claude tool result (<N chars>)'
- system → 'claude system: <subtype>'
- result → 'claude result: <first 200 chars>'
- error → 'claude error: <msg>' (WARNING level)

Makes stalls pinpointable to a specific tool call rather than leaving a
silent gap in the log.
@FidoCanCode FidoCanCode merged commit 4cf84dd into main Apr 15, 2026
3 checks passed
@FidoCanCode FidoCanCode deleted the fix-webhook-reply-tools-and-more-color branch April 15, 2026 00:06
FidoCanCode added a commit that referenced this pull request Apr 15, 2026
Extend the no-tools clause from #517 (reply prompts) to every other prompt
that runs through session.prompt(): triage classifier, needs_more_context
haiku check, summarize-as-action-item, status nudge, rescope/reorder.

Symptom: rhencke/orly PR #32 'Fix CI? <link>' comment got no fido reply
for minutes because the triage classifier opened a Bash shell to
investigate the linked failing CI run instead of just returning a category.
Same pattern across all the other text-only prompts — opus sees a
directive-shaped request and decides to do real work, holding the session
lock and starving the worker.

Adds prompts.NO_TOOLS_CLAUSE constant; prepends it to every classifier/
summarizer/status/rescope prompt.
FidoCanCode added a commit that referenced this pull request Apr 15, 2026
## Summary
- Adds `prompts.NO_TOOLS_CLAUSE` constant and prepends it to every
text-only session prompt: `triage_prompt`, `status_system_prompt`,
`rescope_prompt`, `needs_more_context`, `_summarize_as_action_item`
(both initial + shorten-loop).
- Extends the same fix pattern from #517 (which only banned tools in
reply prompts) to the rest of the session-nudge prompts.

Symptom: rhencke/orly PR #32 "Fix CI? <link>" comment got no fido reply
for minutes because the triage classifier fired Bash calls to
investigate the linked CI run instead of returning a category. Holds the
session lock while opus grinds.

## Test plan
- [x] 1844 tests, 100% coverage, ruff + pyright clean (pre-commit hook)
- [ ] After merge: triage / status / rescope turns return in seconds, no
Bash/Read/Edit calls in the kennel log inside those windows

Closes #528.

Co-authored-by: Fido Can Code <190991155+FidoCanCode@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants