Skip to content

jira: Fix global_read support in availability check and tool handler#20

Merged
cgwalters merged 1 commit into
LobsterTrap:mainfrom
cgwalters:jira-fixes
Apr 16, 2026
Merged

jira: Fix global_read support in availability check and tool handler#20
cgwalters merged 1 commit into
LobsterTrap:mainfrom
cgwalters:jira-fixes

Conversation

@cgwalters

@cgwalters cgwalters commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

In which I actually try using JIRA through this for the first time...

The JIRA availability check required explicit project entries even when global_read was true, making the service report as unavailable. The tool handler also read struct fields directly instead of using accessor methods that fall back to environment variables (JIRA_HOST, JIRA_USERNAME, JIRA_API_TOKEN), so tokens provided via _FILE env vars were ignored.

The project list guard had the same issue, requiring non-empty projects instead of checking has_any_read_access().

Assisted-by: OpenCode (Claude Opus 4)

Summary by CodeRabbit

  • Bug Fixes

    • Improved JIRA project list availability and permission checking to verify actual read access rather than relying on configuration presence alone.
    • Updated error messages to more accurately reflect JIRA read access requirements.
  • Refactor

    • Enhanced JIRA configuration handling with automatic fallback to environment variables when direct configuration is unavailable.

In which I actually try using JIRA through this for the first time...

The JIRA availability check required explicit project entries even when
global_read was true, making the service report as unavailable. The tool
handler also read struct fields directly instead of using accessor methods
that fall back to environment variables (JIRA_HOST, JIRA_USERNAME,
JIRA_API_TOKEN), so tokens provided via _FILE env vars were ignored.

The project list guard had the same issue, requiring non-empty projects
instead of checking has_any_read_access().

Assisted-by: OpenCode (Claude Opus 4)
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters enabled auto-merge April 16, 2026 01:06
@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Refactored src/mcp.rs to use config.jira accessor methods instead of direct pattern-matching, updated JIRA client initialization to use plain token strings, replaced project-configuration permission checks with has_any_read_access() validation, and adjusted availability gating and error messages accordingly.

Changes

Cohort / File(s) Summary
JIRA Configuration & Client Setup
src/mcp.rs
Updated configuration access from direct option pattern-matching to config.jira accessor methods with fallbacks to environment variables; changed JIRA client initialization to use plain token strings instead of expose_secret() on SecureString types.
Permission & Availability Checks
src/mcp.rs
Shifted permission gating for project list command from "at least one projects entry" to has_any_read_access(); refactored check_jira_availability to evaluate read access across projects/issues/global scopes and updated corresponding error messages and status details.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A rabbit hops through JIRA's gate,
No secrets exposed, just tokens straight,
Configuration flows through methods divine,
Access checks sing—read perms align!
Refactored and swift, permission takes flight.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly summarizes the main change: fixing global_read support in JIRA's availability check and tool handler, which is the primary objective of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/mcp.rs (1)

3000-3001: Consider clarifying the missing configuration message.

When has_read_access is false, the message says "project configuration" is missing. However, users could also enable access via global_read: true. Consider changing to "read access configuration" or adding a hint about global_read.

💡 Suggested improvement
         if !has_read_access {
-            missing.push("project configuration");
+            missing.push("read access (global_read or project entries)");
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/mcp.rs` around lines 3000 - 3001, The current branch that pushes "project
configuration" into the missing vector when has_read_access is false is
ambiguous; update the message pushed into missing (where has_read_access is
checked) to something like "read access configuration (or enable global_read)"
or otherwise append a hint about global_read so users know they can enable
global_read: true as an alternative. Locate the check around has_read_access and
replace the literal "project configuration" with a clearer string such as "read
access configuration (or enable global_read)" or programmatically add an extra
hint entry when global_read is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/mcp.rs`:
- Around line 3000-3001: The current branch that pushes "project configuration"
into the missing vector when has_read_access is false is ambiguous; update the
message pushed into missing (where has_read_access is checked) to something like
"read access configuration (or enable global_read)" or otherwise append a hint
about global_read so users know they can enable global_read: true as an
alternative. Locate the check around has_read_access and replace the literal
"project configuration" with a clearer string such as "read access configuration
(or enable global_read)" or programmatically add an extra hint entry when
global_read is available.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0cf7e383-ee44-4182-94f9-947a3387e6a5

📥 Commits

Reviewing files that changed from the base of the PR and between cb08042 and 5737ecf.

📒 Files selected for processing (1)
  • src/mcp.rs

@cgwalters cgwalters merged commit b69814b into LobsterTrap:main Apr 16, 2026
2 checks passed
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.

1 participant