-
Notifications
You must be signed in to change notification settings - Fork 0
feat(cli): T3 Features - Logs, MCP Bridge, Monitor, TUI Enhancements #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
T3 Features Implemented: 1. `fgp logs <service>` - View daemon logs - Tail last N lines with -n flag - Follow mode with -f flag - Color-coded by log level 2. `fgp mcp serve|install|tools` - MCP Bridge - Expose FGP daemons as MCP servers - Auto-register with Claude Code - Meta-tools for daemon management 3. `fgp monitor` - Health notifications - Watch services for state changes - macOS system notifications via osascript - Configurable check interval 4. TUI Dashboard enhancements - [R] key to restart selected service - [d]/Enter to view service details (methods, uptime, version) - Updated help overlay with new shortcuts 5. Workflow templates - `fgp workflow list --builtin` - List bundled templates - `fgp workflow init <template>` - Copy template to ~/.fgp/workflows/ Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1bceb0cc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/commands/mcp_bridge.rs
Outdated
| let result = serde_json::json!({ | ||
| "tools": tools | ||
| }); | ||
|
|
||
| json_rpc_response(None, result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Echo tools/list request id in JSON-RPC response
The MCP bridge uses JSON-RPC 2.0 over stdio, which requires responses to include the same id as the request. handle_tools_list always calls json_rpc_response(None, result), so the response id becomes null. Clients that correlate responses by id (common for JSON-RPC tooling) will treat this as an unmatched response and may hang or fail to populate tool listings. This will occur for any tools/list call made by MCP clients expecting a proper JSON-RPC response.
Useful? React with 👍 / 👎.
src/commands/mcp_bridge.rs
Outdated
| // Parse tool name to extract daemon and method | ||
| // Format: fgp_<daemon>_<method> | ||
| let parts: Vec<&str> = tool_name.strip_prefix("fgp_").unwrap_or(tool_name).splitn(2, '_').collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid ambiguous tool name parsing for daemon names with '_'
Tool names are encoded as fgp_<daemon>_<method> but parsing uses splitn(2, '_'), which splits on the first underscore after the prefix. If a daemon name itself contains underscores (e.g., my_service), the parsed daemon becomes my and the method becomes service_<method>, causing calls to target the wrong socket or fail entirely. This breaks MCP tool calls for any daemon whose service directory name contains underscores.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements 6 T3 features for the FGP CLI, adding logging, monitoring, MCP bridge integration, TUI enhancements, and workflow templates to improve daemon management and developer experience.
Changes:
- Added
logscommand for viewing daemon logs with follow mode and color-coded output - Added
mcpbridge commands to expose FGP daemons as MCP servers for Claude Desktop/Cline - Added
monitorcommand for health monitoring with macOS notifications - Enhanced TUI with restart functionality (
Rkey) and service detail view (d/Enter keys) - Added workflow template system with built-in templates and init commands
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tui/ui.rs | Adds detail overlay rendering and updates footer with new shortcuts |
| src/tui/mod.rs | Implements key binding logic for detail/restart features with overlay state handling |
| src/tui/app.rs | Adds restart_selected, toggle_detail, and load_service_methods functionality |
| src/notifications.rs | New module for macOS system notifications via osascript |
| src/main.rs | Integrates new commands (logs, mcp, monitor) and workflow subcommands |
| src/commands/workflow.rs | Adds built-in templates with list and init subcommands |
| src/commands/monitor.rs | New health monitoring daemon with state tracking and notifications |
| src/commands/mod.rs | Registers new command modules (logs, mcp_bridge, monitor) |
| src/commands/mcp_bridge.rs | New MCP bridge server implementing JSON-RPC 2.0 over stdio |
| src/commands/logs.rs | New logs viewer with tail and follow functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/tui/app.rs
Outdated
| } | ||
|
|
||
| // Brief delay for cleanup | ||
| std::thread::sleep(std::time::Duration::from_millis(100)); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded sleep delay for restart creates a race condition. The 100ms delay may not be sufficient for all services to fully stop before restarting. Consider polling the service status or making the delay configurable.
src/commands/monitor.rs
Outdated
| fn fgp_services_dir() -> PathBuf { | ||
| let base = shellexpand::tilde("~/.fgp/services"); | ||
| PathBuf::from(base.as_ref()) | ||
| } | ||
|
|
||
| /// Get the socket path for a service. | ||
| fn service_socket_path(service: &str) -> PathBuf { | ||
| fgp_services_dir().join(service).join("daemon.sock") | ||
| } |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These helper functions duplicate existing implementations in src/commands/mod.rs. Consider importing and using the shared functions fgp_services_dir() and service_socket_path() from the parent module instead.
src/commands/mcp_bridge.rs
Outdated
| fn fgp_services_dir() -> PathBuf { | ||
| let base = shellexpand::tilde("~/.fgp/services"); | ||
| PathBuf::from(base.as_ref()) | ||
| } | ||
|
|
||
| /// Get the socket path for a service. | ||
| fn service_socket_path(service: &str) -> PathBuf { | ||
| fgp_services_dir().join(service).join("daemon.sock") | ||
| } |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These helper functions duplicate existing implementations in src/commands/mod.rs. Consider importing and using the shared functions fgp_services_dir() and service_socket_path() from the parent module instead.
src/commands/mcp_bridge.rs
Outdated
| return json_rpc_error(id, -32603, &format!("Failed to start daemon: {}", e)); | ||
| } | ||
| // Wait for daemon to be ready | ||
| std::thread::sleep(std::time::Duration::from_millis(500)); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded 500ms sleep after auto-starting a daemon may not be sufficient for slower services to become ready. Consider polling the daemon's health endpoint or making the wait time configurable.
src/notifications.rs
Outdated
| let title = title.replace('"', "\\\""); | ||
| let message = message.replace('"', "\\\""); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The escape handling only addresses double quotes but doesn't handle other shell metacharacters that could be exploited. Consider using proper shell escaping or passing arguments in a way that prevents command injection (e.g., using Command::args() with pre-split values rather than shell evaluation).
src/commands/mcp_bridge.rs
Outdated
| let parts: Vec<&str> = tool_name.strip_prefix("fgp_").unwrap_or(tool_name).splitn(2, '_').collect(); | ||
|
|
||
| if parts.len() != 2 { | ||
| return json_rpc_error(id, -32602, "Invalid tool name format"); | ||
| } | ||
|
|
||
| let daemon = parts[0]; | ||
| let method = parts[1].replace('_', "."); |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No validation is performed on the daemon name extracted from the tool name before using it in file path construction. Consider validating that the daemon name contains only safe characters to prevent potential path traversal vulnerabilities.
src/commands/logs.rs
Outdated
| /// Print a log line with color-coding by level. | ||
| fn print_log_line(line: &str) { | ||
| // Try to detect log level from the line | ||
| let colored_line = if line.contains("ERROR") || line.contains("\"level\":\"ERROR\"") { |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple string contains checks per line can be inefficient for high-volume log output. Consider using a single pass with early return or regex matching for better performance.
Gemini CLI export: - Generates extension directory with gemini-extension.json + GEMINI.md - Compatible with `gemini extensions install` command Aider export: - Generates CONVENTIONS.md file - Load via `aider --read CONVENTIONS.md` Total export targets: 8 (claude-code, cursor, codex, mcp, windsurf, zed, gemini, aider) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- mcp_bridge: Fix JSON-RPC id echo in tools/list response - mcp_bridge: Use double underscore (__) separator to handle daemon names with underscores - mcp_bridge: Add daemon name validation to prevent path traversal - mcp_bridge: Replace hardcoded sleep with health endpoint polling for auto-start - mcp_bridge: Use shared helpers from parent module (fgp_services_dir, service_socket_path) - monitor: Use shared helpers from parent module - notifications: Add proper AppleScript escaping for backslashes and quotes - logs: Optimize log level detection with single-pass approach - logs: Fix clippy warning (filter_map -> map_while) - tui/app: Replace hardcoded restart delay with service stop polling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- skill.rs: Fix needless_borrow and single_match warnings - skill.rs: Fix format_in_format_args warnings - Apply cargo fmt to all modified files Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- notifications.rs: Add newline/carriage return escaping for AppleScript - logs.rs: Use allocation-free case-insensitive search for log levels Previous commits already addressed: - JSON-RPC response id echoing (P1) - Tool name parsing with double underscore separator (P2) - Shared helper function usage - Daemon name validation for path traversal prevention - Health polling instead of hardcoded sleeps Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Implements all 6 T3 features for the FGP CLI:
fgp logs <service>- View daemon logs with tail/follow modes and color-coded outputfgp mcp serve|install|tools- MCP bridge exposing FGP daemons for Claude Desktop/Cline compatibilityfgp monitor- Health monitoring with macOS notifications on state changesRkey to restart,d/Enter for service detail view with methods list--builtinlist andinitcommandsTest plan
fgp logs gmail(requires a running daemon with logs)fgp mcp toolsto list available MCP toolsfgp tui, press?to see new shortcuts, pressdon a servicefgp workflow list --builtinto see templatesfgp monitor --interval 10to test health monitoring🤖 Generated with Claude Code