dagu icon indicating copy to clipboard operation
dagu copied to clipboard

Model Context Protocol (MCP) server implementation

Open SiwonP opened this issue 3 weeks ago • 3 comments

Implementation of an MCP server for Dagu #976

Summary by CodeRabbit

  • New Features
    • Adds an "mcp" CLI command to run an MCP server.
    • Provides tools to list DAGs, list DAG runs, and fetch a DAG.
    • Runs over standard I/O for easy integration with pipelines.
    • Prevents concurrent runs via directory locking and respects runtime options from config/env.
    • Exposes a CLI entrypoint to start and manage the MCP server.

✏️ Tip: You can customize this high-level summary in your review settings.

SiwonP avatar Dec 04 '25 12:12 SiwonP

Walkthrough

Adds a new "mcp" CLI subcommand wired into the root command, exposes Context.NewMcpServer(), introduces an MCPServer type with New and Start methods (with directory locking and tool registration), and updates go.mod to add an MCP SDK and a URI template library.

Changes

Cohort / File(s) Summary
CLI entry
\cmd/main.go``
Registers the new Mcp() subcommand on the root command.
CLI command implementation
\internal/cmd/mcp.go``
Adds Mcp() *cobra.Command, mcpFlags placeholder, and runMcp(ctx *Context, args []string) error which constructs and starts the MCP server via Context.
Context extension
\internal/cmd/context.go``
Adds func (c *Context) NewMcpServer() (*mcpserver.MCPServer, error) to construct MCP server instances.
MCP server service
\internal/service/mcpserver/mcpserver.go``
New MCPServer type with New(cfg *config.Config) (*MCPServer, error) and Start(ctx context.Context) error; initializes time/location, configures a directory lock, registers tools (list_DAGs, list_DAG_runs, get_DAG), and serves via stdio transport.
Dependency updates
\go.mod``
Adds github.com/modelcontextprotocol/go-sdk v1.1.0 and github.com/yosida95/uritemplate/v3 v3.0.2 (and adjusts indirect deps).

Sequence Diagram

sequenceDiagram
    participant User as CLI User
    participant Root as rootCmd
    participant McpCmd as Mcp Command
    participant Runner as runMcp()
    participant Ctx as Context
    participant MCP as MCPServer
    participant Lock as DirLock
    participant Stdio as StdioTransport
    participant API as Local API

    User->>Root: run "mcp"
    Root->>McpCmd: execute
    McpCmd->>Runner: invoke runMcp(ctx)
    Runner->>Ctx: NewMcpServer()
    Ctx->>MCP: New(cfg)
    MCP-->>Runner: MCPServer instance
    Runner->>MCP: Start(ctx)
    MCP->>Lock: acquire lock on DAGs dir
    alt lock acquired
        MCP->>Stdio: serve over stdio transport
        Stdio->>MCP: deliver tool requests
        MCP->>API: fetch DAGs / DAG runs / DAG details
        API-->>MCP: return data
        MCP-->>Stdio: return tool responses
        MCP-->>Runner: Start returns nil
    else lock failed
        MCP-->>Runner: Start returns error
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30–40 minutes

  • Review focus:
    • internal/service/mcpserver/mcpserver.go — constructor, Start(), directory lock acquisition/release, tool registration, and error handling.
    • internal/cmd/mcp.go and internal/cmd/context.go — CLI wiring and Context API.
    • go.mod — ensure added dependencies are actually used and correctly versioned.

Poem

🐇 I nibble code and hop about,

A tiny "mcp" command to shout,
I lock the DAGs and list them fine,
I serve by stdio, one request at a time,
Hooray — the rabbit twitches, all aligned!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an MCP server implementation to the codebase, which is reflected across all modified files.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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 and usage tips.

coderabbitai[bot] avatar Dec 04 '25 12:12 coderabbitai[bot]

  • I would recommend removing the hardcoded values in listDags(). The function currently has hardcoded host, port, and api_base_url with commented-out environment variable lookups. These should either use environment variables or be configurable through the MCPServer struct to avoid brittleness.
  • It looks like the mcpFlags slice is empty in mcp.go. Consider adding relevant flags (e.g., for configuring the host, port, or other MCP server settings) if they're needed, or document why no flags are necessary.
  • I would recommend avoiding the use of context.Background() in the Start() method. Since the method receives a ctx parameter, you should pass that context to s.server.Run() instead of creating a fresh background context, which will ignore cancellation signals from the caller.
  • It looks like the commented-out executeDag() and createDag() functions should either be completed and uncommented or removed entirely to reduce code clutter and maintenance burden.
  • I would recommend storing the config field in the MCPServer struct (it's passed to New() but never stored), as you may need it later for accessing configuration values or passing to other methods.
  • It looks like the running atomic field is declared but never used. Consider removing it if it's not needed, or implement start/stop logic if it is meant to track the server's state.

jyroscoped avatar Dec 05 '25 00:12 jyroscoped

Codecov Report

:x: Patch coverage is 0.70423% with 141 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 59.58%. Comparing base (b4c857d) to head (ffc296c). :warning: Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
internal/service/mcpserver/mcpserver.go 0.00% 122 Missing :warning:
internal/cmd/mcp.go 0.00% 17 Missing :warning:
internal/cmd/context.go 0.00% 2 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1444      +/-   ##
==========================================
- Coverage   59.95%   59.58%   -0.38%     
==========================================
  Files         191      193       +2     
  Lines       21473    21615     +142     
==========================================
+ Hits        12875    12880       +5     
- Misses       7249     7386     +137     
  Partials     1349     1349              
Files with missing lines Coverage Δ
cmd/main.go 86.95% <100.00%> (+0.59%) :arrow_up:
internal/cmd/context.go 70.12% <0.00%> (-0.62%) :arrow_down:
internal/cmd/mcp.go 0.00% <0.00%> (ø)
internal/service/mcpserver/mcpserver.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b4c857d...ffc296c. Read the comment docs.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 11 '25 01:12 codecov[bot]