Add repo workflow tools to the MCP server

index c2af613..491c112 100644 --- a/.agents/specs/mcp-server.md +++ b/.agents/specs/mcp-server.md @@ -1,6 +1,6 @@ --- name: mcp-server -description: "Specification for the siGit MCP server at /api/v1/mcp. A stateless Streamable-HTTP JSON-RPC endpoint that exposes siGit repositories to MCP clients (Claude Code and others) as six tools: list_repositories, get_file_contents, search_code, list_pull_requests, create_pull_request, add_issue_comment. Covers transport, auth, the protocol method set, the tool contracts, the pull request / issue / comment data model, authorization, and acceptance criteria." +description: "Specification for the siGit MCP server at /api/v1/mcp. A stateless Streamable-HTTP JSON-RPC endpoint that exposes siGit repositories to MCP clients (Claude Code and others) as ten tools: list_repositories, get_file_contents, search_code, list_issues, get_issue, create_issue, list_pull_requests, get_pull_request, create_pull_request, add_issue_comment. Covers transport, auth, the protocol method set, the tool contracts, the pull request / issue / comment data model, authorization, and acceptance criteria." status: implemented implements: - app/controllers/api/v1/mcp_controller.rb @@ -8,11 +8,13 @@ implements: - app/services/mcp/tools.rb - app/services/mcp/tool_error.rb - app/models/{pull_request,issue,comment,repository}.rb - - app/services/{pull_request_service,comment_service}.rb + - app/services/{pull_request_service,issue_service,comment_service}.rb - config/routes.rb verifies: - spec/requests/api/v1/mcp_spec.rb + - spec/requests/api/v1/mcp_tools_spec.rb - spec/services/pull_request_service_spec.rb + - spec/services/issue_service_spec.rb - spec/services/comment_service_spec.rb --- @@ -142,7 +144,7 @@ so adding a tool is a one-line change (append the class to `all`). Each tool declares `tool_name`, `description`, `input_schema` (JSON Schema surfaced to the model), and a `call`. -The six tools, in registry order: +The ten tools, in registry order: ### list_repositories @@ -177,6 +179,35 @@ The six tools, in registry order: parses `<ref>:<path>:<line>:<text>`. - **Output:** array of `{ path, line, snippet }`, capped at `limit`. +### list_issues + +- **Purpose:** list issues, optionally by state or a text query. +- **Input (required):** `repo`. Optional `state` in `open|closed|all` (default + `open`), `query` (case-insensitive substring of the issue title or body). +- **Behaviour:** `repo.issues`, filtered by state unless `all`, `ILIKE` filter + on title or body when `query` is given, ordered by `number` descending, + limited to 50. +- **Output:** array of `{ number, title, state, author, url }`. + +### get_issue + +- **Purpose:** read one issue in full, including its comments. +- **Input (required):** `repo`, `number`. +- **Behaviour:** resolve the repo (read scope), find the issue by number. +- **Output:** `{ number, title, state, author, body, url, comments }` where + `comments` is an array of `{ author, body, created_at }`, oldest first. +- **Errors (in band):** no issue with that number. + +### create_issue + +- **Purpose:** open a new issue. +- **Input (required):** `repo`, `title`. Optional `body` (Markdown). +- **Behaviour:** routed through `IssueService.create` so per-repo numbering + (shared with pull requests) runs identically to any other caller. Anyone who + can read the repo may open an issue on it, as with comments. +- **Output:** `{ number, state, url }`. +- **Errors (in band):** blank title; any model validation failure. + ### list_pull_requests - **Purpose:** list pull requests, optionally by state. @@ -186,6 +217,20 @@ The six tools, in registry order: `number` descending, limited to 50. - **Output:** array of `{ number, title, state, head, base, url }`. +### get_pull_request + +- **Purpose:** read one pull request in full, including its comments and diff. +- **Input (required):** `repo`, `number`. +- **Behaviour:** resolve the repo (read scope), find the PR by number, then + `Repository#diff_between(base_ref, head_ref)` -> `GitRepositoryService.diff` + (three-dot `git diff base...head`, the range a pull request shows). +- **Output:** `{ number, title, state, author, head, base, body, url, comments, + diff }`. The diff is capped at 100 kB; past the cap the result carries + `diff_truncated: true` and a `diff_note`. When a ref no longer exists (e.g. + the head branch of a merged PR was deleted), `diff` is null with a + `diff_note` saying so. +- **Errors (in band):** no pull request with that number. + ### create_pull_request - **Purpose:** open a pull request from a head branch into a base branch. @@ -224,31 +269,35 @@ following were added to back the tools. and a user (author). Column: `body` (present). `html_url` anchors to the parent. - **Shared numbering.** Pull requests and issues draw from one per-repository number space (as on GitHub), so a number maps to exactly one record. The next - number is `max(pull_requests.number, issues.number) + 1`, computed under a - repository row lock during creation. + number is `max(pull_requests.number, issues.number) + 1`, computed by + `Repository#next_issue_number` under a repository row lock during creation; + both `PullRequestService` and `IssueService` allocate through it. ## Authorization -- **Read** (list_repositories, get_file_contents, search_code, list_pull_requests, - resolving the repo for any tool): `Repository.visible_to(user)`, which returns - public repositories plus the caller's own. Private repositories of other users - are never resolvable, so they cannot be read or mutated. +- **Read** (list_repositories, get_file_contents, search_code, list_issues, + get_issue, list_pull_requests, get_pull_request, resolving the repo for any + tool): `Repository.visible_to(user)`, which returns public repositories plus + the caller's own. Private repositories of other users are never resolvable, + so they cannot be read or mutated. - **Write** (create_pull_request): `Repository#writable_by?(user)`. siGit repos have a single owner and no collaborators yet, so write equals ownership. A non-owner attempt returns an in-band `isError`. -- **Comment** (add_issue_comment): requires read access to the repo (enforced by - repo resolution). The author is always the authenticated user. +- **Comment and issue creation** (add_issue_comment, create_issue): require read + access to the repo (enforced by repo resolution). The author is always the + authenticated user. -Mutations go through the service objects (`PullRequestService`, `CommentService`), -never directly through `create!` from the tool, so that the same validations and -authorization run for the MCP path as for any future web UI. +Mutations go through the service objects (`PullRequestService`, `IssueService`, +`CommentService`), never directly through `create!` from the tool, so that the +same validations and authorization run for the MCP path as for any future web UI. ## Supporting layers - **Git read layer.** `Repository#blob_at(ref, path)` delegates to `GitRepositoryService.file_content` (a `git show <ref>:<path>`). `Repository#search_code` delegates to `GitRepositoryService.search_code` - (`git grep`). + (`git grep`). `Repository#diff_between(base, head)` delegates to + `GitRepositoryService.diff` (`git diff base...head`). - **URLs.** `Repository#html_url`, `PullRequest#html_url`, `Issue#html_url`, and `Comment#html_url` build absolute links from `Repository.site_url`, which reads `SIGITSI_URL` (default `https://sigit.si`). This is needed because tools have no @@ -258,7 +307,7 @@ authorization run for the MCP path as for any future web UI. 1. `claude mcp add --transport http sigit https://sigit.si/api/v1/mcp --header "Authorization: Bearer <token>"` connects. -2. `tools/list` returns all six tools. +2. `tools/list` returns all ten tools. 3. `list_repositories` and `get_file_contents` work end to end against a real repository. 4. An unauthenticated request returns a JSON-RPC 401 with a `WWW-Authenticate` @@ -268,12 +317,16 @@ authorization run for the MCP path as for any future web UI. ## Verification - Request specs (`spec/requests/api/v1/mcp_spec.rb`): initialize handshake, - `tools/list` shows all six, a successful `tools/call`, a notification answered + `tools/list` shows all ten, a successful `tools/call`, a notification answered with 202, a 401 with the challenge, and a tool returning `isError`. +- Request specs (`spec/requests/api/v1/mcp_tools_spec.rb`): the issue and pull + request tools end to end against a real bare repository — happy paths + (including the real three-dot diff), diff truncation, missing-branch diffs, + unknown numbers, an invisible repository, and missing required arguments. - Service specs (`spec/services/pull_request_service_spec.rb`, - `comment_service_spec.rb`): authorization, branch validation, head-differs-from- - base, shared numbering, comment resolution across issues and pull requests, - blank-body rejection. + `issue_service_spec.rb`, `comment_service_spec.rb`): authorization, branch + validation, head-differs-from-base, shared numbering, comment resolution + across issues and pull requests, blank-title and blank-body rejection. ## Out of scope (current) and natural extensions @@ -283,8 +336,8 @@ authorization run for the MCP path as for any future web UI. - Webhooks on pull request / issue / comment creation. None exist in the app yet; when added, they run automatically because mutations go through the service objects. -- Further tools, each a one-line registry addition: `get_pull_request` (with - diff), `list_branches`, `list_commits`, `create_issue`, `list_issues`. +- Further tools, each a one-line registry addition: `list_branches`, + `list_commits`, `close_issue`, `merge_pull_request`. ## Key files @@ -292,8 +345,8 @@ authorization run for the MCP path as for any future web UI. - `app/services/mcp/server.rb`: protocol dispatch. - `app/services/mcp/tools.rb`: tool registry and tool implementations. - `app/services/mcp/tool_error.rb`: in-band tool failure type. -- `app/services/pull_request_service.rb`, `app/services/comment_service.rb`: - mutation services. +- `app/services/pull_request_service.rb`, `app/services/issue_service.rb`, + `app/services/comment_service.rb`: mutation services. - `app/models/pull_request.rb`, `issue.rb`, `comment.rb`, and the additions to `repository.rb` and `git_repository_service.rb`. - `config/routes.rb`: `post`/`get api/v1/mcp`.

index d60dbcd..82a8bfa 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -62,6 +62,22 @@ class Repository < ApplicationRecord GitRepositoryService.search_code(disk_path, ref || default_branch, query, limit: limit) end + # Unified diff of +head+ against its merge base with +base+ (what a pull + # request shows). Returns the diff String ("" when the refs are identical), + # or nil when either ref is unknown — e.g. the head branch of a merged pull + # request has been deleted. + def diff_between(base, head) + GitRepositoryService.diff(disk_path, base, head) + end + + # Next number in the shared issue / pull request number space. Issues and + # pull requests draw from one per-repository sequence (as on GitHub), so a + # number resolves to exactly one record. Call inside a `with_lock` block + # when allocating. + def next_issue_number + [ pull_requests.maximum(:number) || 0, issues.maximum(:number) || 0 ].max + 1 + end + def git_path disk_path end

index 536b557..b895c60 100644 --- a/app/services/git_repository_service.rb +++ b/app/services/git_repository_service.rb @@ -198,6 +198,16 @@ class GitRepositoryService end end + # Unified diff of +head+ against its merge base with +base+ (three-dot + # `git diff base...head`, the same range a pull request shows). Returns the + # diff String ("" when the refs are identical), or nil when either ref is + # unknown or the repository is missing. + def self.diff(path, base, head) + out, _err, status = Open3.capture3("git", "--git-dir", path, "diff", "#{base}...#{head}") + return nil unless status.success? + out + end + def self.branch_exists?(path, branch) _out, _err, status = Open3.capture3("git", "--git-dir", path, "rev-parse", "--verify", branch) status.success?

new file mode 100644 index 0000000..a260277 --- /dev/null +++ b/app/services/issue_service.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# Creates issues. All issue creation — web UI or MCP — goes through here so +# per-repo numbering (shared with pull requests) behaves identically across +# callers. +class IssueService + class Error < StandardError; end + + # Opens an issue on +repository+ authored by +author+. Anyone who can read a + # repository may open an issue on it, so read access is enforced by the + # caller (the MCP layer only resolves repos the user can read), as with + # comments. Raises ActiveRecord::RecordInvalid on validation failure. + def self.create(repository:, author:, title:, body: nil) + repository.with_lock do + repository.issues.create!( + user: author, + number: repository.next_issue_number, + title: title, + body: body.to_s, + state: "open" + ) + end + end +end

index cc5d2a4..4ef5cb9 100644 --- a/app/services/mcp/tools.rb +++ b/app/services/mcp/tools.rb @@ -19,7 +19,11 @@ module Mcp ListRepositories, GetFileContents, SearchCode, + ListIssues, + GetIssue, + CreateIssue, ListPullRequests, + GetPullRequest, CreatePullRequest, AddIssueComment ].freeze @@ -71,6 +75,14 @@ module Mcp .find_by(users: { username: owner }, repositories: { name: name }) repo || raise(Mcp::ToolError, "Repository not found or not accessible: #{full_name}") end + + # Comments serialized oldest-first — the shape both get_issue and + # get_pull_request return. + def comments_for(commentable) + commentable.comments.includes(:user).order(:created_at).map do |c| + { "author" => c.author.username, "body" => c.body, "created_at" => c.created_at.iso8601 } + end + end end # ----------------------------------------------------------------------- # @@ -154,6 +166,88 @@ module Mcp end end + class ListIssues < Base + name! "list_issues" + describe "List issues in a repository (requires repo), optionally filtered by state or a title/body search." + input_schema( + "type" => "object", + "required" => %w[repo], + "properties" => { + "repo" => { "type" => "string", "description" => "Repository in 'owner/name' form." }, + "query" => { "type" => "string", "description" => "Optional case-insensitive substring of the issue title or body." }, + "state" => { "type" => "string", "enum" => %w[open closed all], "default" => "open" } + } + ) + + def call(args) + repo = find_repo!(require_arg(args, "repo")) + state = args["state"].presence || "open" + scope = state == "all" ? repo.issues : repo.issues.where(state: state) + if args["query"].present? + scope = scope.where("issues.title ILIKE :q OR issues.body ILIKE :q", q: "%#{args['query']}%") + end + scope.includes(:user).order(number: :desc).limit(50).map do |issue| + { "number" => issue.number, "title" => issue.title, "state" => issue.state, + "author" => issue.author.username, "url" => issue.html_url } + end + end + end + + class GetIssue < Base + name! "get_issue" + describe "Fetch one issue by number (requires repo and number), including its body and comments." + input_schema( + "type" => "object", + "required" => %w[repo number], + "properties" => { + "repo" => { "type" => "string", "description" => "Repository in 'owner/name' form." }, + "number" => { "type" => "integer", "description" => "Issue number." } + } + ) + + def call(args) + repo = find_repo!(require_arg(args, "repo")) + number = require_arg(args, "number") + issue = repo.issues.find_by(number: number) + raise Mcp::ToolError, "No issue ##{number} in #{repo.full_name}." unless issue + + { "number" => issue.number, "title" => issue.title, "state" => issue.state, + "author" => issue.author.username, "body" => issue.body, "url" => issue.html_url, + "comments" => comments_for(issue) } + end + end + + class CreateIssue < Base + name! "create_issue" + describe "Open a new issue in a repository (requires repo and title)." + input_schema( + "type" => "object", + "required" => %w[repo title], + "properties" => { + "repo" => { "type" => "string", "description" => "Repository in 'owner/name' form." }, + "title" => { "type" => "string" }, + "body" => { "type" => "string", "description" => "Issue description (Markdown)." } + } + ) + + def call(args) + repo = find_repo!(require_arg(args, "repo")) + # Route through the service so per-repo numbering (shared with pull + # requests) runs exactly as it does for any other caller. + issue = IssueService.create( + repository: repo, + author: current_user, + title: require_arg(args, "title"), + body: args["body"] + ) + { "number" => issue.number, "state" => issue.state, "url" => issue.html_url } + rescue IssueService::Error => e + raise Mcp::ToolError, e.message + rescue ActiveRecord::RecordInvalid => e + raise Mcp::ToolError, e.record.errors.full_messages.to_sentence + end + end + class ListPullRequests < Base name! "list_pull_requests" describe "List pull requests in a repository, optionally filtered by state." @@ -177,6 +271,51 @@ module Mcp end end + class GetPullRequest < Base + # Unified diffs are included up to this many bytes so one large pull + # request cannot blow out the model's context window. + DIFF_MAX_BYTES = 100_000 + + name! "get_pull_request" + describe "Fetch one pull request by number (requires repo and number), " \ + "including its body, comments, and unified diff (truncated past #{DIFF_MAX_BYTES / 1000} kB)." + input_schema( + "type" => "object", + "required" => %w[repo number], + "properties" => { + "repo" => { "type" => "string", "description" => "Repository in 'owner/name' form." }, + "number" => { "type" => "integer", "description" => "Pull request number." } + } + ) + + def call(args) + repo = find_repo!(require_arg(args, "repo")) + number = require_arg(args, "number") + pr = repo.pull_requests.find_by(number: number) + raise Mcp::ToolError, "No pull request ##{number} in #{repo.full_name}." unless pr + + { "number" => pr.number, "title" => pr.title, "state" => pr.state, + "author" => pr.author.username, "head" => pr.head_ref, "base" => pr.base_ref, + "body" => pr.body, "url" => pr.html_url, + "comments" => comments_for(pr) }.merge(diff_fields(repo, pr)) + end + + private + + # The three-dot diff of head against base, capped at DIFF_MAX_BYTES. + # nil with a note when a ref no longer exists (e.g. the head branch of a + # merged pull request was deleted). + def diff_fields(repo, pr) + diff = repo.diff_between(pr.base_ref, pr.head_ref) + return { "diff" => nil, "diff_note" => "Diff unavailable: a branch no longer exists." } if diff.nil? + return { "diff" => diff } if diff.bytesize <= DIFF_MAX_BYTES + + { "diff" => diff.byteslice(0, DIFF_MAX_BYTES).scrub(""), + "diff_truncated" => true, + "diff_note" => "Diff truncated to the first #{DIFF_MAX_BYTES} bytes." } + end + end + class CreatePullRequest < Base name! "create_pull_request" describe "Open a pull request from a head branch into a base branch."

index 39fdc0c..7ce5cc1 100644 --- a/app/services/pull_request_service.rb +++ b/app/services/pull_request_service.rb @@ -22,7 +22,9 @@ class PullRequestService repository.with_lock do repository.pull_requests.create!( user: author, - number: next_number(repository), + # Issues and pull requests share one per-repo number space (as on + # GitHub); Repository#next_issue_number is the single allocator. + number: repository.next_issue_number, title: title, head_ref: head, base_ref: base, @@ -31,12 +33,4 @@ class PullRequestService ) end end - - # Issues and pull requests share one per-repo number space (as on GitHub), so a - # number resolves to exactly one of them. Call inside a repository row lock. - def self.next_number(repository) - [ repository.pull_requests.maximum(:number) || 0, - repository.issues.maximum(:number) || 0 ].max + 1 - end - private_class_method :next_number end

index 56628a5..15d2378 100644 --- a/spec/requests/api/v1/mcp_spec.rb +++ b/spec/requests/api/v1/mcp_spec.rb @@ -61,13 +61,14 @@ RSpec.describe "Api::V1::Mcp", type: :request do end describe "tools/list" do - it "lists all six tools" do + it "lists all ten tools" do tools = rpc({ "jsonrpc" => "2.0", "id" => 2, "method" => "tools/list" })["result"]["tools"] names = tools.map { |t| t["name"] } expect(names).to contain_exactly( "list_repositories", "get_file_contents", "search_code", - "list_pull_requests", "create_pull_request", "add_issue_comment" + "list_issues", "get_issue", "create_issue", + "list_pull_requests", "get_pull_request", "create_pull_request", "add_issue_comment" ) expect(tools).to all(include("description", "inputSchema")) end

new file mode 100644 index 0000000..a932c64 --- /dev/null +++ b/spec/requests/api/v1/mcp_tools_spec.rb @@ -0,0 +1,232 @@ +# frozen_string_literal: true + +require "rails_helper" +require "tmpdir" +require "fileutils" + +# Request specs for the repo-workflow MCP tools (issues and pull requests). +# Each tool is exercised end to end through POST /api/v1/mcp: the happy path, +# a repository the caller cannot see, and missing required arguments — the +# failures reported in-band as isError, never as protocol errors. +RSpec.describe "Api::V1::Mcp repo workflow tools", type: :request do + around do |example| + Dir.mktmpdir("mcp-tools-spec") do |tmp| + @repo_dir = File.join(tmp, "demo.git") + build_repo_with_feature_branch(@repo_dir) + example.run + end + end + + let(:user) do + User.create!(smbcloud_id: 4243, email: "mcp-tools-tester@example.com", username: "mcptools") + end + let(:stranger) do + User.create!(smbcloud_id: 4244, email: "stranger@example.com", username: "stranger") + end + + let!(:repository) do + user.repositories.create!( + name: "demo", kind: "code", default_branch: "main", disk_path: @repo_dir + ) + end + + # A private repository owned by someone else: invisible to the caller, so + # every tool must answer "not found or not accessible" rather than leak it. + let!(:private_repo) do + stranger.repositories.create!( + name: "secret", kind: "code", default_branch: "main", is_private: true, + disk_path: GitRepositoryService.repo_path("stranger", "secret") + ) + end + + let(:token) { "valid-access-token" } + let(:auth_headers) do + { "Authorization" => "Bearer #{token}", "Content-Type" => "application/json" } + end + + before do + allow_any_instance_of(Api::V1::McpController) + .to receive(:resolve_user) { |_controller, presented| presented == token ? user : nil } + end + + def call_tool(name, arguments) + post "/api/v1/mcp", + params: { "jsonrpc" => "2.0", "id" => 1, "method" => "tools/call", + "params" => { "name" => name, "arguments" => arguments } }.to_json, + headers: auth_headers + response.parsed_body["result"] + end + + # Successful tool output is a JSON document in the text content block. + def payload(result) + expect(result["isError"]).to be(false) + JSON.parse(result["content"].first["text"]) + end + + def error_text(result) + expect(result["isError"]).to be(true) + result["content"].first["text"] + end + + describe "list_issues" do + before do + repository.issues.create!(user: user, number: 1, title: "Auth bug", body: "Login fails on Safari") + repository.issues.create!(user: user, number: 2, title: "Update docs", state: "closed") + end + + it "lists open issues by default" do + issues = payload(call_tool("list_issues", { "repo" => "mcptools/demo" })) + expect(issues.map { |i| i["number"] }).to eq([ 1 ]) + expect(issues.first).to include("title" => "Auth bug", "state" => "open", "author" => "mcptools") + end + + it "filters by state and by a title/body query" do + issues = payload(call_tool("list_issues", + { "repo" => "mcptools/demo", "state" => "all", "query" => "docs" })) + expect(issues.map { |i| i["number"] }).to eq([ 2 ]) + end + + it "rejects a repository the caller cannot see" do + result = call_tool("list_issues", { "repo" => "stranger/secret" }) + expect(error_text(result)).to match(/not found or not accessible/i) + end + + it "rejects a call missing the repo argument" do + result = call_tool("list_issues", {}) + expect(error_text(result)).to match(/Missing required argument: repo/) + end + end + + describe "get_issue" do + let!(:issue) do + repository.issues.create!(user: user, number: 1, title: "Auth bug", body: "Login fails") + end + + it "returns the issue with its body and comments" do + issue.comments.create!(user: stranger, body: "Reproduced on my machine") + + data = payload(call_tool("get_issue", { "repo" => "mcptools/demo", "number" => 1 })) + expect(data).to include("number" => 1, "title" => "Auth bug", "state" => "open", + "author" => "mcptools", "body" => "Login fails") + expect(data["comments"]).to contain_exactly( + a_hash_including("author" => "stranger", "body" => "Reproduced on my machine") + ) + end + + it "reports an unknown issue number in-band" do + result = call_tool("get_issue", { "repo" => "mcptools/demo", "number" => 99 }) + expect(error_text(result)).to match(/No issue #99/) + end + + it "rejects a repository the caller cannot see" do + result = call_tool("get_issue", { "repo" => "stranger/secret", "number" => 1 }) + expect(error_text(result)).to match(/not found or not accessible/i) + end + + it "rejects a call missing the number argument" do + result = call_tool("get_issue", { "repo" => "mcptools/demo" }) + expect(error_text(result)).to match(/Missing required argument: number/) + end + end + + describe "create_issue" do + it "opens an issue numbered in the shared issue/PR space" do + repository.pull_requests.create!( + user: user, number: 1, title: "PR", head_ref: "feature", base_ref: "main" + ) + + data = payload(call_tool("create_issue", + { "repo" => "mcptools/demo", "title" => "Flaky spec", "body" => "Fails on CI" })) + expect(data).to include("number" => 2, "state" => "open") + + issue = repository.issues.find_by(number: 2) + expect(issue.title).to eq("Flaky spec") + expect(issue.body).to eq("Fails on CI") + expect(issue.author).to eq(user) + end + + it "rejects a repository the caller cannot see" do + result = call_tool("create_issue", { "repo" => "stranger/secret", "title" => "X" }) + expect(error_text(result)).to match(/not found or not accessible/i) + end + + it "rejects a call missing the title argument" do + result = call_tool("create_issue", { "repo" => "mcptools/demo" }) + expect(error_text(result)).to match(/Missing required argument: title/) + end + end + + describe "get_pull_request" do + let!(:pull_request) do + repository.pull_requests.create!( + user: user, number: 1, title: "Add feature", body: "Adds the feature", + head_ref: "feature", base_ref: "main" + ) + end + + it "returns the pull request with its comments and unified diff" do + pull_request.comments.create!(user: stranger, body: "LGTM") + + data = payload(call_tool("get_pull_request", { "repo" => "mcptools/demo", "number" => 1 })) + expect(data).to include("number" => 1, "title" => "Add feature", "state" => "open", + "head" => "feature", "base" => "main", "author" => "mcptools") + expect(data["comments"]).to contain_exactly(a_hash_including("author" => "stranger", "body" => "LGTM")) + expect(data["diff"]).to include("feature.rb").and include("+puts 'shiny new feature'") + expect(data).not_to have_key("diff_truncated") + end + + it "truncates an oversized diff and says so" do + big_diff = "+x" * Mcp::Tools::GetPullRequest::DIFF_MAX_BYTES + allow_any_instance_of(Repository).to receive(:diff_between).and_return(big_diff) + + data = payload(call_tool("get_pull_request", { "repo" => "mcptools/demo", "number" => 1 })) + expect(data["diff_truncated"]).to be(true) + expect(data["diff"].bytesize).to eq(Mcp::Tools::GetPullRequest::DIFF_MAX_BYTES) + expect(data["diff_note"]).to match(/truncated/i) + end + + it "returns a nil diff with a note when a branch no longer exists" do + pull_request.update!(head_ref: "deleted-branch") + + data = payload(call_tool("get_pull_request", { "repo" => "mcptools/demo", "number" => 1 })) + expect(data["diff"]).to be_nil + expect(data["diff_note"]).to match(/branch no longer exists/i) + end + + it "reports an unknown pull request number in-band" do + result = call_tool("get_pull_request", { "repo" => "mcptools/demo", "number" => 99 }) + expect(error_text(result)).to match(/No pull request #99/) + end + + it "rejects a repository the caller cannot see" do + result = call_tool("get_pull_request", { "repo" => "stranger/secret", "number" => 1 }) + expect(error_text(result)).to match(/not found or not accessible/i) + end + + it "rejects a call missing the number argument" do + result = call_tool("get_pull_request", { "repo" => "mcptools/demo" }) + expect(error_text(result)).to match(/Missing required argument: number/) + end + end + + # A bare repo whose `feature` branch adds one file on top of `main`, so the + # three-dot diff a pull request shows is small and predictable. + def build_repo_with_feature_branch(bare_path) + FileUtils.mkdir_p(bare_path) + system("git", "init", "--bare", bare_path, exception: true) + Dir.mktmpdir do |work| + system("git", "-C", work, "init", "-b", "main", exception: true) + system("git", "-C", work, "config", "user.email", "t@example.com", exception: true) + system("git", "-C", work, "config", "user.name", "Test", exception: true) + File.write(File.join(work, "README.md"), "# Demo\n") + system("git", "-C", work, "add", ".", exception: true) + system("git", "-C", work, "commit", "-m", "seed", exception: true) + system("git", "-C", work, "checkout", "-b", "feature", exception: true) + File.write(File.join(work, "feature.rb"), "puts 'shiny new feature'\n") + system("git", "-C", work, "add", ".", exception: true) + system("git", "-C", work, "commit", "-m", "add feature", exception: true) + system("git", "-C", work, "remote", "add", "origin", bare_path, exception: true) + system("git", "-C", work, "push", "origin", "main", "feature", exception: true) + end + end +end

new file mode 100644 index 0000000..edda2fe --- /dev/null +++ b/spec/services/issue_service_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe IssueService do + let(:owner) { User.create!(smbcloud_id: 9001, email: "owner@example.com", username: "owner") } + let(:reader) { User.create!(smbcloud_id: 9002, email: "reader@example.com", username: "reader") } + let(:repo) do + owner.repositories.create!( + name: "app", kind: "code", default_branch: "main", + disk_path: GitRepositoryService.repo_path("owner", "app") + ) + end + + it "opens an issue authored by the given user" do + issue = described_class.create(repository: repo, author: owner, title: "Bug", body: "Details") + + expect(issue).to be_persisted + expect(issue.number).to eq(1) + expect(issue.state).to eq("open") + expect(issue.author).to eq(owner) + expect(issue.body).to eq("Details") + end + + it "lets any reader open an issue, not only the owner" do + issue = described_class.create(repository: repo, author: reader, title: "Feature request") + expect(issue.author).to eq(reader) + end + + it "shares one number space with pull requests" do + repo.pull_requests.create!(user: owner, number: 3, title: "PR", head_ref: "feature", base_ref: "main") + issue = described_class.create(repository: repo, author: owner, title: "Bug") + expect(issue.number).to eq(4) + end + + it "rejects a blank title via model validation" do + expect do + described_class.create(repository: repo, author: owner, title: "") + end.to raise_error(ActiveRecord::RecordInvalid) + end +end