feat: add MCP issue search - #187
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 3 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Previously reported and still active
Resolved this pass
Could not review: tinysweeper/description Before merge
How this fits togetherflowchart LR
n0["GitHubRead<br/>changed<br/>4 findings"]:::blocking
n1["McpState<br/>changed"]:::changed
n2["build_router<br/>changed"]:::changed
n3["call<br/>changed"]:::changed
n4["search_code<br/>changed"]:::changed
n5["Result"]:::impacted
n6["new"]:::impacted
n7["create_issue"]:::impacted
n8["github"]:::impacted
n9["search_issues"]:::impacted
n2 -->|uses| n1
n2 -->|calls| n6
n3 -->|uses| n0
n3 -->|uses| n1
n3 -->|calls| n4
n3 -->|uses| n5
n3 -->|calls| n6
n3 -->|calls| n7
n3 -->|uses| n8
n3 -->|calls| n9
n4 -->|uses| n1
n4 -->|uses| n5
n4 -->|calls| n6
n6 -->|uses| n5
n7 -->|uses| n0
n7 -->|uses| n1
n7 -->|uses| n5
n7 -->|calls| n6
n7 -->|uses| n8
n7 -->|calls| n9
n9 -->|uses| n0
n9 -->|uses| n1
n9 -->|uses| n5
n9 -->|calls| n6
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
Warning Review limit reached
This review includes 3 billable files and costs up to $0.75. Or wait 41 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0151 · 220,730 in / 7,314 out · 29,714 cached (13%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 384 embedded
critique: $0.0101 · 143,247 in / 4,456 out · 26,019 cached (18%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0031 · 44,621 in / 1,834 out · 3,695 cached (8%) · gpt-5.6-luna
description: $0.0005 · 8,418 in / 61 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0011 · 20,015 in / 108 out · 0 cached (0%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p1
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f024507b51
ℹ️ 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".
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a157b8739
ℹ️ 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".
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0233 · 347,615 in / 14,142 out · 18,157 cached (5%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 668 embedded
critique: $0.0134 · 198,960 in / 8,244 out · 12,682 cached (6%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0063 · 87,352 in / 4,154 out · 5,475 cached (6%) · gpt-5.6-luna
tests: $0.0011 · 20,125 in / 121 out · 0 cached (0%) · deepseek/deepseek-v4-flash
description: $0.0006 · 10,744 in / 162 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0013 · 22,963 in / 194 out · 0 cached (0%) · deepseek/deepseek-v4-flash
Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fb004a0bf
ℹ️ 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".
| if query.split_whitespace().any(|word| { | ||
| let word = word.to_ascii_lowercase(); | ||
| word.starts_with("repo:") || word.starts_with("org:") || word.starts_with("user:") |
There was a problem hiding this comment.
A query such as bug OR (repo:other-org/private-repo parser) passes this check because split_whitespace() produces (repo:other-org/private-repo, which does not start with repo:. GitHub issue search accepts qualifiers inside parenthesized Boolean expressions, so the resulting scoped query can still search another repository and then emit URLs using the requested repository. Fresh evidence in this revision is the unhandled parenthesized form; normalize leading Boolean punctuation or parse and reject all scope qualifiers before searching.
Useful? React with 👍 / 👎.
| if scoped.len() > 256 { | ||
| return Err(Error::Forge( | ||
| "issue search exceeds GitHub's 256-byte query limit".into(), |
There was a problem hiding this comment.
create_issue passes its title to this shared search method before checking force, so adding is:issue plus this 256-byte guard rejects part of the advertised 256-byte title range. For example, a 224-byte ASCII title for tinyhumansai/teeny passed the previous 248-byte scoped query, but the new qualifier makes it 257 bytes and prevents issue creation even with force: true; construct a separately bounded duplicate-search query rather than rejecting an otherwise valid issue title.
Useful? React with 👍 / 👎.
| let token = read_token(state, repo).await?; | ||
| let forge = crate::forge::github::GitHubRead::new(&token).map_err(|error| error.to_string())?; | ||
| search_issues_with(&forge, repo, args).await |
There was a problem hiding this comment.
Every production search_issues call reaches this line only after call has already invoked read_token and constructed a GitHubRead for canonicalization. Calling read_token again performs another uncached installation_for_repo GitHub request (even when the token itself is cached), adding latency and consuming API capacity on every issue search; pass the existing read client into the handler instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0090 · 104,288 in / 12,930 out · 7,608 cached (7%) · ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 711 embedded
critique: $0.0025 · 32,920 in / 2,590 out · 4,048 cached (12%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0020 · 27,236 in / 1,727 out · 3,560 cached (13%) · gpt-5.6-luna
tests: $0.0027 · 17,113 in / 6,399 out · 0 cached (0%) · deepseek/deepseek-v4-flash
e2e: $0.0012 · 19,948 in / 732 out · 0 cached (0%) · deepseek/deepseek-v4-flash
| // Callers exposing GitHub's query language to untrusted input must | ||
| // reject scope qualifiers first. The MCP tool does so before reaching | ||
| // this shared adapter; internal callers retain the full search syntax. | ||
| let scoped = format!( |
There was a problem hiding this comment.
Avoid silently breaking internal callers of search_issues
The is:issue qualifier is now appended unconditionally in the shared search_issues adapter. Internal callers such as src/sentry/dedupe.rs and src/bin/tinysweeper.rs (via backfill_discussions) use search_issues and may rely on it returning both issues and pull requests. Making this change at the lowest shared level without auditing all callers introduces a latent correctness bug for any non-MCP caller that needs PR results. The MCP tool should append is:issue at its own level (in search_issues_with or in a wrapper), not in the forge adapter.
Additional tests observation
Cover the new query construction and error path with a test
[RULE] untested-behaviour-change
The adapter now prepends is:issue and enforces a 256‑byte limit on the composed query. Neither behaviour is exercised by any test: the mock forge’s search_issues does substring matching without recording or validating the query, and the length‑check branch is never hit. A regression that removed the is:issue qualifier or dropped the length check would not cause a test failure. Add a unit test for GitHubRead::search_issues (or an integration test against a recorded fixture) that verifies the query contains is:issue and that a long query is rejected with the correct error.
Suggested change for this observation (reference only)
[RULE] breaking-contract-change ·
| repo.name, | ||
| query.trim() | ||
| ); | ||
| if scoped.len() > 256 { |
There was a problem hiding this comment.
Cover the 256-byte query rejection end to end
The new 256-byte truncation guard is a behavioural boundary in the forge adapter. The MCP server now exposes a tool that reaches this code, yet no end-to-end test drives a query that exceeds 256 bytes. A test that sends a 257-byte query and expects a structured error response would confirm the guard works and that clients see a meaningful error rather than a cryptic failure from GitHub.
Additional critique observation
Count query characters instead of bytes
[RULE] incorrect-length-unit
str::len() counts UTF-8 bytes, while GitHub's search limit is expressed in characters. For example, a query containing 200 Japanese characters is 600 bytes but only 200 characters, so this adapter rejects a search GitHub accepts. Use a character count for the limit (or otherwise match the API's character-based contract).
Suggested change for this observation (reference only)
if scoped.chars().count() > 256 {
[RULE] blocking-missing-e2e ·
1 participant
Summary
search_issuesMCP tool over the existing repository-scoped GitHub searchVerification
cargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warningscargo test --locked --features serve server::mcp::(13 passed)