Skip to content

fix(desktop): keep agent hook debug logs off the PTY so they can't corrupt TUIs - #5634

Open
AviPeltz wants to merge 1 commit into
mainfrom
fix/notify-hook-stderr-tui
Open

AviPeltz wants to merge 1 commit into
mainfrom
fix/notify-hook-stderr-tui

Conversation

@AviPeltz

@AviPeltz AviPeltz commented Jul 12, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Agent lifecycle hooks (notify-hook, gemini-hook) were writing debug/diagnostic lines to stderr. Hooks run as children of TUI agents (OpenCode, Claude, Gemini) with stdio inherited from the PTY, so those lines printed at the cursor position and corrupted the TUI's diff-rendered screen — interleaved [notify-hook] lines and orphaned text fragments overlaying the UI.
  • All diagnostics now go exclusively to the existing debug_log file channel (/tmp/superset-agent-hooks.log, overridable via SUPERSET_HOOK_DEBUG_LOG). No hook template writes to stdout/stderr anymore (gemini-hook still prints its protocol-required {} JSON to stdout).

Why / Context

notify-hook.template.sh auto-enables debug output when SUPERSET_ENV=development, and its stderr echoes were exact duplicates of the debug_log file writes next to them. In dev builds, every agent lifecycle event (Start/Stop/PermissionRequest) sprayed log lines into the agent's TUI. Diff-rendering TUIs never recover from foreign writes: they repaint only cells they believe changed, so hook text persists as garbage and bottom-row writes desync their screen model by one line. gemini-hook.template.sh additionally had an ungated stderr echo on its v2→v1 fallback path that fired even in production.

How It Works

  • notify-hook.template.sh: dropped the three stderr echoes; the event-received debug_log line now carries the full field set the echo had (adds paneId, workspaceId). The v1 dispatch was duplicated into debug/non-debug branches solely so debug mode could echo the status code — collapsed to a single branch that always captures the status and debug_logs it.
  • gemini-hook.template.sh: the fallback echo now appends to the same log file (unconditional, since it's a rare failure-path signal).
  • New regression test asserts no hook template contains >&2.

Debugging workflow is unchanged in capability: tail -f /tmp/superset-agent-hooks.log instead of reading interleaved (and mangled) lines inside the TUI.

Manual QA

  • Rendered the template ({{MARKER}}/{{DEFAULT_PORT}} substituted) and executed it with a fake Stop payload:
    • with SUPERSET_DEBUG_HOOKS=1: stdout+stderr empty, log file receives the event line (full field set) and dispatch status line
    • without debug: stdout+stderr empty
  • bash -n on both modified templates.
  • Rollout is automatic: createNotifyScript() rewrites the installed notify.sh on next app launch via content comparison (no marker bump needed — hook semantics unchanged, wire payloads identical).

Testing

  • bun test src/main/lib/agent-setup (73 pass, includes new no-stderr regression test)
  • bun run lint
  • bun run typecheck (desktop)

Known Limitations

  • OpenCode's payload fields (hookSessionId/resourceId) parse as empty in the notify hook — pre-existing and unrelated to the logging channel; the Stop event still dispatches. Worth a separate ticket.

Open in Stage

Summary by cubic

Keep agent hook debug output off the PTY to stop corrupting diff-rendered TUIs. All diagnostics now go to the hook debug log file instead of stdout/stderr.

  • Bug Fixes
    • Removed all stderr echoes from notify-hook.template.sh and gemini-hook.template.sh; diagnostics append to ${SUPERSET_HOOK_DEBUG_LOG:-/tmp/superset-agent-hooks.log}. Gemini still prints only the required JSON to stdout.
    • Simplified notify-hook v1 dispatch into a single path and logged status with more context fields (paneId, workspaceId, etc.).
    • Added a regression test to ensure no hook template writes to stderr (>&2) and updated expectations for the notify script content.

Written for commit 819d943. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Prevented agent hook diagnostics from appearing unexpectedly in terminal output or interfering with command-line protocols.
    • Improved logging for host-service dispatch failures, including response status details and event context.
    • Preserved fallback behavior when host-service delivery is unavailable, while recording failures in the debug log.

…rrupt TUIs

Hooks run as children of TUI agents (opencode, claude, gemini) with stdio
inherited from the PTY, so the dev-mode stderr echoes in notify-hook (and
an ungated failure echo in gemini-hook) printed at the cursor position and
corrupted diff-rendered screens with interleaved log fragments.

Route all diagnostics to the existing debug_log file channel
(/tmp/superset-agent-hooks.log), fold the duplicated debug/non-debug v1
dispatch branches into one, and add a regression test that no hook
template writes to stderr.
@ghost

ghost commented Jul 12, 2026

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 3 individual chapters for you:

Title
1 Redirect notify-hook diagnostics to debug log file
2 Fix gemini-hook fallback logging to avoid PTY corruption
3 Verify hook output constraints with regression tests
Open in Stage

Chapters generated by Stage for commit 819d943 on Jul 12, 2026 5:15pm UTC.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 146c6964-84ac-4049-b36b-4a8e9a07feb1

📥 Commits

Reviewing files that changed from the base of the PR and between 20191d6 and 819d943.

📒 Files selected for processing (3)
  • apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
  • apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.sh
  • apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh

📝 Walkthrough

Walkthrough

Hook templates now route diagnostics to configurable log files instead of stdout or stderr, while retaining host-service and v1 fallback dispatches. Tests cover detailed debug payloads and prevent stderr redirection.

Changes

Hook diagnostic logging

Layer / File(s) Summary
Template dispatch logging
apps/desktop/src/main/lib/agent-setup/templates/*.template.sh
Notify and Gemini hooks record dispatch statuses and fallback notices in debug log files without emitting stderr output.
Logging behavior tests
apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
Tests verify detailed event context in debug logs and reject stderr redirection in hook templates.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: keeping agent hook debug logs off the PTY to avoid TUI corruption.
Description check ✅ Passed The description covers summary, context, implementation, manual QA, and testing; only optional template sections are omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/notify-hook-stderr-tui

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR moves agent hook diagnostics out of the TUI PTY. The main changes are:

  • Notify hook debug output now goes through the file-backed debug log.
  • Notify hook v1 dispatch now uses one status-capturing curl path.
  • Gemini fallback diagnostics now append to the debug log instead of stderr.
  • A regression test checks hook templates for direct stderr writes.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh Moves notify-hook diagnostics to debug_log and keeps v1/v2 dispatch behavior intact.
apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.sh Moves Gemini fallback diagnostics from stderr to the debug log while preserving required stdout output.
apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts Updates debug-log expectations and adds coverage for direct stderr redirection in hook templates.

Reviews (1): Last reviewed commit: "fix(desktop): keep agent hook debug logs..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 3 files

Re-trigger cubic

This branch has not been deployed

No deployments
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.

1 participant