Skip to content

feat: implement Monitor tool for streaming shell output#649

Merged
kevincodex1 merged 3 commits into
Gitlawb:mainfrom
Flo5k5:feat/implement-monitor-tool
Apr 13, 2026
Merged

feat: implement Monitor tool for streaming shell output#649
kevincodex1 merged 3 commits into
Gitlawb:mainfrom
Flo5k5:feat/implement-monitor-tool

Conversation

@Flo5k5
Copy link
Copy Markdown
Contributor

@Flo5k5 Flo5k5 commented Apr 12, 2026

Summary

Implement the Monitor tool — a new tool that executes shell commands in the background and streams stdout line-by-line as notifications to the model. This is the last missing tool implementation from the source snapshot.

What Monitor does

Monitor fills the gap between synchronous Bash and fire-and-forget run_in_background:

Mode Blocking Notifications
Bash Yes Immediate result
Bash(run_in_background) No 1 notification at completion
Monitor No Streaming (~1s polling)

Use cases: watching logs (tail -f), monitoring builds, observing dev server startup.

Implementation (3 new files + 1 flag flip)

src/tools/MonitorTool/MonitorTool.ts (195 lines)

  • Input: { command, description }
  • Spawns a LocalShellTask with kind: 'monitor' via exec() + spawnShellTask()
  • Returns immediately with { taskId, outputFile }
  • Permission checking delegates to bashToolHasPermission
  • The existing task polling infrastructure handles streaming output to the model

src/tasks/MonitorMcpTask/MonitorMcpTask.ts (99 lines)

  • Registers monitor_mcp task type in the task registry (satisfies import from tasks.ts)
  • Exports killMonitorMcpTasksForAgent() for agent cleanup (called from runAgent.ts:866)
  • Architecture: MonitorTool creates local_bash tasks with kind='monitor'; the monitor_mcp type exists for forward-compatibility with MCP-based monitoring

src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.tsx (170 lines)

  • Permission dialog showing command + description
  • "Don't ask again" creates a command-prefix rule (like BashTool), not a blanket allow

scripts/build.tsMONITOR_TOOL: true

Pre-existing infrastructure used

The codebase already had all integration points wired:

  • src/tools.ts:39-41 — MonitorTool import slot
  • src/tasks.ts:12-14 — MonitorMcpTask import slot
  • src/components/permissions/PermissionRequest.tsx:40-41,73-74 — permission mapping
  • src/tasks/LocalShellTask/LocalShellTask.tsx:47kind: 'monitor' (stall watchdog disabled)
  • src/tasks/LocalShellTask/LocalShellTask.tsx:129-143 — monitor-specific notifications
  • src/tools/BashTool/prompt.ts — conditional Monitor guidance text
  • src/Task.ts:12'monitor_mcp' in TaskType

Self-review fixes (commit 2)

  • Permission: "don't ask again" now creates command-prefix rule instead of blanket tool-name rule
  • Architecture comments clarified in MonitorMcpTask

Test plan

  • bun run build — compiles successfully
  • bun run smoke — smoke tests pass
  • Self-review: permission rule, architecture docs, integration points verified
  • Run Monitor({ command: "echo hello && sleep 1 && echo world", description: "test" }) — verify streaming notifications
  • Verify permission dialog appears and "don't ask again" creates command-specific rule
  • Verify TaskStop can kill a running monitor
  • Verify monitor tasks are cleaned up on agent exit

Flo5k5 added 2 commits April 13, 2026 01:12
Add the Monitor tool that executes shell commands in the background and
streams stdout line-by-line as notifications to the model. This enables
real-time monitoring of logs, builds, and long-running processes.

Implementation:
- MonitorTool (src/tools/MonitorTool/) — spawns LocalShellTask with
  kind='monitor', returns immediately with task ID
- MonitorMcpTask (src/tasks/MonitorMcpTask/) — task lifecycle management
  and agent cleanup via killMonitorMcpTasksForAgent()
- MonitorPermissionRequest — permission dialog component

The codebase already had all integration points wired (tools.ts, tasks.ts,
PermissionRequest.tsx, LocalShellTask kind='monitor', BashTool prompt).
This PR provides the missing implementations.
- MonitorPermissionRequest: "don't ask again" now creates a
  command-prefix rule (like BashTool) instead of a blanket
  tool-name-only rule that would auto-allow all Monitor commands
- MonitorMcpTask: clarify architecture comments explaining why
  monitor_mcp type exists as a registry stub while actual tasks
  are local_bash with kind='monitor'
Copilot AI review requested due to automatic review settings April 12, 2026 23:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Monitor tool to run shell commands asynchronously while streaming stdout back to the model via periodic task-output notifications, filling the gap between blocking Bash and completion-only background tasks.

Changes:

  • Introduces MonitorTool that spawns LocalShellTask jobs with kind: 'monitor' and returns { taskId, outputFile } immediately.
  • Registers a forward-compatible monitor_mcp task type and adds agent-exit cleanup for monitor-related tasks.
  • Adds a dedicated permission dialog for Monitor and enables the MONITOR_TOOL feature flag in the open build.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
src/tools/MonitorTool/MonitorTool.ts Implements the Monitor tool entrypoint that spawns background shell tasks in monitor mode and maps the immediate result payload.
src/tasks/MonitorMcpTask/MonitorMcpTask.ts Adds the monitor_mcp task registry entry plus agent-scoped cleanup for monitor-related tasks.
src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.tsx Adds the Monitor-specific permission prompt UI and “don’t ask again” flow.
scripts/build.ts Enables the MONITOR_TOOL feature flag for the open build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.tsx Outdated
Comment thread src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.tsx Outdated
Comment thread src/components/permissions/MonitorPermissionRequest/MonitorPermissionRequest.tsx Outdated
Comment thread src/tools/MonitorTool/MonitorTool.ts
Comment thread src/tools/MonitorTool/MonitorTool.ts
Comment thread src/tasks/MonitorMcpTask/MonitorMcpTask.ts Outdated
- Fix permission rule field: expression → ruleContent (Copilot #1)
- Handle empty command prefix: skip rule creation (Copilot Gitlawb#2)
- Remove unused useTheme() import (Copilot Gitlawb#3)
- Save permission rules under 'Bash' toolName so bashToolHasPermission
  can match them — Monitor delegates to Bash permission system (Copilot Gitlawb#4)
- Remove unused logError import from MonitorMcpTask (Copilot Gitlawb#6)
- Copilot Gitlawb#5 (getAppState throws): same pattern as BashTool:915, not a bug
Flo5k5 added a commit to Flo5k5/openclaude that referenced this pull request Apr 13, 2026
With KAIROS=true, MONITOR_TOOL=true, and COORDINATOR_MODE=true, the build
system's auto-generated noop stubs are truthy — causing runtime crashes when
included in tool/task arrays. Add proper null-export stubs so conditional
checks (?:) correctly exclude them.

Stubs added:
- MonitorTool, MonitorMcpTask, MonitorMcpDetailDialog, MonitorPermissionRequest
  (will be replaced by PR Gitlawb#649's full implementation)
- SendUserFileTool, PushNotificationTool (internal tools, no open-build impl)
- coordinator/workerAgent (will be replaced by PR Gitlawb#647's implementation)
Flo5k5 added a commit to Flo5k5/openclaude that referenced this pull request Apr 13, 2026
…nt (Gitlawb#647)

Replace null stubs with full implementations from PRs Gitlawb#649 and Gitlawb#647:

- MonitorTool: streaming shell output tool with bash permission delegation,
  background task spawning, and 30min timeout (PR Gitlawb#649)
- MonitorMcpTask: task registry entry with agent-scoped cleanup for
  monitor-kind shell tasks (PR Gitlawb#649)
- MonitorPermissionRequest: permission UI with "don't ask again" support,
  delegates rules to Bash toolName for correct matching (PR Gitlawb#649)
- workerAgent: WORKER_AGENT definition + getCoordinatorAgents() returning
  worker, general-purpose, explore, and plan agents (PR Gitlawb#647)

MonitorMcpDetailDialog remains a null stub (not in PR Gitlawb#649's scope).
Copy link
Copy Markdown
Collaborator

@Vasanthdev2004 Vasanthdev2004 left a comment

Choose a reason for hiding this comment

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

Review: Monitor tool for streaming shell output

Reviewed on head a9c7206. CI green ✅. 4 files, +471/-1.

This is a clean implementation of the Monitor tool — fills the last missing tool slot from the source snapshot. The architecture is sound: it correctly reuses the existing LocalShellTask infrastructure with kind: 'monitor' for streaming notifications, delegates permissions to bashToolHasPermission, and registers the forward-compatible monitor_mcp task type.

✅ Architecture

The tool sits correctly between synchronous Bash and fire-and-forget run_in_background:

Mode Blocking Notifications
Bash Yes Immediate result
Bash(run_in_background) No 1 notification at completion
Monitor No Streaming (~1s polling)

All pre-existing integration points are properly filled: tools.ts import, tasks.ts import, PermissionRequest.tsx mapping, LocalShellTask monitor-kind handling, BashTool/prompt.ts conditional guidance, Task.ts type definition.

✅ Permission system

  • checkPermissions correctly delegates to bashToolHasPermission — Monitor runs shell commands, so the same rules apply
  • "Don't ask again" correctly saves rules under toolName: 'Bash' (not 'Monitor'), because bashToolHasPermission checks rules against BashTool. Saving under 'Monitor' would create a dead rule that never matches. The code comment explains this clearly.
  • Prefix extraction uses first 2 tokens → ${prefix}:* format, matching BashTool's shellRuleMatching pattern
  • Empty/whitespace commands skip rule creation entirely (empty array passed to onAllow) — prevents wildcard-everything rules

✅ Self-review fixes (commit 2)

All 6 Copilot issues addressed in a9c7206:

  1. expressionruleContent (correct schema field) ✅
  2. Empty command guard (skip rule creation) ✅
  3. toolName: 'Bash' for permission rules ✅
  4. Removed unused useTheme()
  5. Removed unused logError import ✅
  6. getAppState throwing stub — kept for consistency with BashTool pattern ✅

✅ Task cleanup

killMonitorMcpTasksForAgent provides belt-and-suspenders cleanup: kills both monitor_mcp-typed tasks AND local_bash tasks with kind='monitor'. The existing killShellTasksForAgent already handles the latter, but being explicit guards against ordering issues. dequeueAllMatching purges queued notifications for the dead agent.

✅ Monitor-specific behavior in LocalShellTask

  • Stall watchdog disabled for kind: 'monitor' — correct, monitor tasks are expected to run indefinitely
  • Distinct notification summaries: "Monitor X stream ended" vs "background command completed" — prevents confusion
  • 30-minute timeout is reasonable for monitor tasks

🟡 Non-blocking observation: test coverage

The PR has no test files. The tool is mostly wiring (delegating to existing infrastructure), so the blast radius of bugs is limited. The permission logic and task cleanup paths are the most testable. This is acceptable for a tool that fills a pre-wired slot, but a follow-up adding tests for MonitorPermissionRequest's rule creation and killMonitorMcpTasksForAgent's cleanup would be valuable.


Verdict: Approve-ready

Clean implementation, addresses all Copilot review feedback, follows established patterns, security properties correct. Kevin already approved. No blockers.

@kevincodex1 kevincodex1 merged commit b818dd5 into Gitlawb:main Apr 13, 2026
1 check passed
C1ph3r404 pushed a commit to C1ph3r404/openclaude that referenced this pull request Apr 29, 2026
* feat: implement Monitor tool for streaming shell output

Add the Monitor tool that executes shell commands in the background and
streams stdout line-by-line as notifications to the model. This enables
real-time monitoring of logs, builds, and long-running processes.

Implementation:
- MonitorTool (src/tools/MonitorTool/) — spawns LocalShellTask with
  kind='monitor', returns immediately with task ID
- MonitorMcpTask (src/tasks/MonitorMcpTask/) — task lifecycle management
  and agent cleanup via killMonitorMcpTasksForAgent()
- MonitorPermissionRequest — permission dialog component

The codebase already had all integration points wired (tools.ts, tasks.ts,
PermissionRequest.tsx, LocalShellTask kind='monitor', BashTool prompt).
This PR provides the missing implementations.

* fix: command-specific permission rule + architecture docs

- MonitorPermissionRequest: "don't ask again" now creates a
  command-prefix rule (like BashTool) instead of a blanket
  tool-name-only rule that would auto-allow all Monitor commands
- MonitorMcpTask: clarify architecture comments explaining why
  monitor_mcp type exists as a registry stub while actual tasks
  are local_bash with kind='monitor'

* fix: address Copilot review feedback

- Fix permission rule field: expression → ruleContent (Copilot #1)
- Handle empty command prefix: skip rule creation (Copilot #2)
- Remove unused useTheme() import (Copilot #3)
- Save permission rules under 'Bash' toolName so bashToolHasPermission
  can match them — Monitor delegates to Bash permission system (Copilot #4)
- Remove unused logError import from MonitorMcpTask (Copilot Gitlawb#6)
- Copilot #5 (getAppState throws): same pattern as BashTool:915, not a bug
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.

4 participants