Skip to content

[AI-62] feat: Add filing-jira-tickets skill to bitwarden-atlassian-tools - #189

Open
SaintPatrck wants to merge 4 commits into
feat/jira-write-tools-mcpfrom
feat/jira-write-tools-skill
Open

[AI-62] feat: Add filing-jira-tickets skill to bitwarden-atlassian-tools#189
SaintPatrck wants to merge 4 commits into
feat/jira-write-tools-mcpfrom
feat/jira-write-tools-skill

Conversation

@SaintPatrck

Copy link
Copy Markdown
Contributor

🎟️ Tracking

AI-62

📔 Objective

Adds the filing-jira-tickets skill to bitwarden-atlassian-tools: it reads a project's create screen before drafting, places acceptance criteria in whatever field the project provides (or the description if it has none), previews each ticket in plain language and takes approval before any live write, then wires and verifies dependency links.

Drives the create_issue/link_issues/get_create_fields MCP tools end to
end: reads a project's create screen before drafting so no project's
field layout is assumed, previews each payload in plain language and
takes approval before any live write, then wires and verifies
dependency links.
Stop routing the link-verification fallback through get_issue, whose
output has no links section to check. Split Gherkin criteria across
descriptionParagraphs entries when there is no criteria field, since
a single paragraph has no line-break node and would render as one
run-on line. Reflect that a failed create screen read can mean either
a missing project or missing permission, matching what
get_create_fields actually reports. Drop get_issue from allowed-tools
now that nothing in the skill calls it.
@SaintPatrck SaintPatrck added the ai-review Request a Claude code review label Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude Configuration Validation — PR #189

Verdict: PASS. No critical or major issues. No blocking errors. The findings below are minor and mostly optional.

[AI-62] feat: Add filing-jira-tickets skill to bitwarden-atlassian-tools

Scope note (affects how to read this report)

This is a stacked PR: base is feat/jira-write-tools-mcp, not main. Its own diff is 6 files:

Status File
modified .claude-plugin/marketplace.json
modified README.md
modified plugins/bitwarden-atlassian-tools/.claude-plugin/plugin.json
modified plugins/bitwarden-atlassian-tools/CHANGELOG.md
modified plugins/bitwarden-atlassian-tools/README.md
added plugins/bitwarden-atlassian-tools/skills/filing-jira-tickets/SKILL.md

The MCP server sources in the task's changed-file list (src/jira/*, src/tools/create-issue.ts, link-issues.ts, get-create-fields.ts, utils/*) belong to the base PR and are already at v2.5.0 here. They were reviewed anyway and are noted as pre-existing where relevant, so the write-tool surface this skill drives got a security pass regardless.

Corrections to automated findings

Two high-severity findings were raised by the plugin-validator and skill-reviewer agents. Both were checked against the official Claude Code skills reference and do not hold. They are recorded here so they are not re-raised:

  1. allowed-tools omitting create_issue / link_issues / AskUserQuestion does NOT break the skill. Per the frontmatter reference, allowed-tools is a pre-approval list: "It does not restrict which tools are available: every tool remains callable, and your permission settings still govern tools that are not listed." The skill is fully operable. The claim that it is "inoperable past Step 2" is incorrect, and commit 166f0dd ("Drop AskUserQuestion; it needs no pre-approval") is correct.

    Consequence: omitting the write tools is a sound deliberate choice, not a defect. Every live create_issue/link_issues call prompts the user, which reinforces the skill's approval-before-create rule (SKILL.md:14, :48). Treat as a positive.

  2. when_to_use is a supported field, not a broken one. Per the same reference, it is "Additional context for when Claude should invoke the skill... Appended to description in the skill listing and counts toward the 1,536-character cap." The trigger phrases on SKILL.md:4 are surfaced for skill selection. Combined description + when_to_use = 732 chars, comfortably under the 1,536 cap. (plugin-dev's own agents/skill-reviewer.md:57 calls the field deprecated; that guidance is stale relative to the current docs. 18 of 59 SKILL.md files in this repo use it.)


1. Plugin Validation — PASS

plugins/bitwarden-atlassian-tools

  • Manifest .claude-plugin/plugin.json: valid JSON; name kebab-case; version 2.6.0 valid semver; description/author/keywords well-formed; no unknown fields.
  • Structure: auto-discovery compliant. README.md, CHANGELOG.md, .gitignore present. No node_modules, .env, or .DS_Store committed. pnpm-lock.yaml committed, as --frozen-lockfile in .mcp.json:8 requires.
  • Commands / Agents / Hooks: none present — not required, and no AGENT.md version bump applies.
  • Skills: 3, all with valid frontmatter and directory names matching name.
  • MCP server .mcp.json: type: "stdio" with command ✓; ${CLAUDE_PLUGIN_ROOT} used for the server path (line 8) ✓; all five env values are ${VAR} placeholders, no literal secrets (lines 11–15) ✓.
  • Credentials: none hardcoded anywhere in the plugin.

Version-bump policy — PASS

Location Version
.claude-plugin/marketplace.json:51 2.6.0
plugins/bitwarden-atlassian-tools/.claude-plugin/plugin.json:3 2.6.0
README.md:12 (root catalog) 2.6.0
agents/*/AGENT.md n/a — no agents

CHANGELOG.md:8-12 has ## [2.6.0] - 2026-08-04 under ### Added, Keep a Changelog format. MINOR is the correct bump for a backward-compatible new skill.

CI pre-checks (run locally)

Check Result
prettier --check . Clean — all matched files use Prettier style
cspell (plugin tree) Clean — no new dictionary entries needed
tsc --noEmit (MCP server) Clean — exit 0
vitest run (MCP server) 15 files, 350 tests passed

2. Skill Review — PASS (minor items)

plugins/bitwarden-atlassian-tools/skills/filing-jira-tickets/SKILL.md — 62 lines, 976 words, well under the 500-line limit.

  • Frontmatter valid: name kebab-case and matches its directory; description present; when_to_use and allowed-tools both supported fields with valid values.
  • MCP tool prefix mcp__plugin_bitwarden-atlassian-tools_bitwarden-atlassian__ matches the sibling skill convention (assessing-jira-issue-relevance/SKILL.md:4).
  • Trigger phrases are specific and third-person; the when_to_use exclusion clause cleanly disambiguates from both siblings (researching-jira-issues owns "understand this issue", assessing-jira-issue-relevance owns "is this still relevant", this one owns creation).
  • No broken references — the body references no files or paths at all.
  • All four MCP tools it names (get_create_fields, create_issue, link_issues, get_issue) exist and are registered (src/index.ts:54-56, 74-76).

Factual accuracy against the tool surface is excellent. Every API claim was verified in source: dry-run defaults (validation.ts:287, :317, :323); the descriptionParagraphs→one-paragraph-with-no-line-break-node constraint (adf-build.ts:39-45); link_issues self-verification and its "could not verify" path (link-issues.ts:73-98); that get_issue renders no links section (utils/format-issue.ts); and the Step 1 404 ambiguity (get-create-fields.ts:119-127).

3. Configuration & Security Review — PASS

  • No committed secrets. Every token-shaped match in the plugin is either a ${VAR} placeholder (.mcp.json:11-15), a README instructional placeholder (README.md:17-22, e.g. "your-jira-write-scoped-token"), or a test fixture ("read-token", "write-token", "Basic dGVzdA=="dGVzdA== decodes to test). No real credentials.
  • No settings.local.json committed; no .claude/ or CLAUDE.md files changed in this PR.
  • Transport security enforced in code: src/jira/auth.ts:61 hardcodes https://api.atlassian.com; src/utils/validation.ts:176-185 pins attachment downloads to *.atlassian.net.
  • No dangerous auto-approvals. allowed-tools grants exactly one read-only MCP tool plus Read — no Bash, no write tools. Least privilege is respected and then some.
  • Injection surface reviewed. JiraProjectKey (validation.ts:219-221, ^[A-Z][A-Z0-9_]+$) is constrained separately from CreateIssueSchema.project precisely because it is interpolated into a createmeta REST path — the path-traversal surface is identified and closed, with a comment saying why. issueTypeId is server-derived from Jira's own response (get-create-fields.ts:152-155), never caller-supplied.

Security design highlights (worth preserving)

  • Layered write safety: dry-run-by-default at the schema level, a separate ATLASSIAN_JIRA_WRITE_TOKEN gate at the handler level (create-issue.ts:122, link-issues.ts:139), shared refusal wording in write-guard.ts, and skill-level approval-before-create. Read and write tokens are distinct env vars (auth.ts:31-34).
  • verifyLink uses a fresh read-mode client (link-issues.ts:79) so verification never requires a read scope on the write token.
  • Link direction cannot be inverted by argument order — role-named blockerKey/blockedKey resolved once in resolveLinkDirection, enforced by a discriminated union.
  • resolveEnv (auth.ts:12-18) treats a literal unexpanded "${VAR}" as missing, so a misconfigured install fails loudly rather than sending a garbage credential.
  • README blast-radius disclosure is exemplary (README.md:128): it states plainly that the write token permits more than these two tools use, names the three over-granted scopes and why Atlassian forces them, and advises rotation.

Findings

All minor. None block merge.

In this PR's diff

# Location Finding Remediation
1 skills/filing-jira-tickets/SKILL.md:33 Warning. Asserts "that field is plain text" for any project's criteria field, justifying a raw Gherkin string. True for PM (customfield_10192 is a textarea, per adf-build.ts:6-9) but not guaranteed elsewhere — and the skill's own thesis is to assume nothing. create_issue passes fields through untouched (create-issue.ts:42) and builds ADF only for the description, so a doc-typed criteria field would reject a raw string. get_create_fields already prints each field's schema type (get-create-fields.ts:57). Instruct the reader to check the type Step 1 reported: send a raw string only for string/textarea types, and fall back to the already-described descriptionParagraphs path for rich text. Impact is bounded — Jira rejects clearly and line 50 already routes back to Step 1.
2 skills/filing-jira-tickets/SKILL.md:18 Warning. Says "QA, VULN, and PLT do not" expose a criteria field. Both other sources say only "QA and VULN do not" (get-create-fields.ts:8-9, plugin README.md:171). SKILL.md is the outlier. Drop PLT from that clause, or trim the whole list to one illustrative example — Step 1 discovers all of it at runtime anyway, so the triplicated facts are pure drift risk.
3 skills/filing-jira-tickets/SKILL.md:41 Suggestion. Step 3 says "in the order given". parentKey is a create-time field (create-issue.ts:53-55), so a child listed before its parent fails outright. Step 4's recovery net covers links only; a parent is not a link type. Add: parents are created before their children regardless of the order given, and parentKey must name an already-created or pre-existing key.
4 skills/filing-jira-tickets/SKILL.md:12 Suggestion. Never mentions ATLASSIAN_JIRA_WRITE_TOKEN. On a read-only install create_issue returns the refusal in write-guard.ts:16-23, making Step 3's completion criterion unreachable with no stated fallback. One sentence: on a read-only install, preview and stop, and tell the user the token is missing.
5 skills/filing-jira-tickets/SKILL.md:5 Suggestion. Read is pre-approved but no step reads a file. Conversely, the sibling-ticket style check on line 31 ("check one if you are unsure") needs get_issue/search_issues, which will prompt. Drop the unused Read. Optionally add get_issue if line 31 is meant to be routine; otherwise soften it to "ask the user for the convention". Note the write tools should stay unlisted — see Correction 1.
6 plugins/bitwarden-atlassian-tools/README.md:151-176 Suggestion. The Skills section documents researching-jira-issues and filing-jira-tickets but omits assessing-jira-issue-relevance, which shipped in 2.4.0. Pre-existing, but this section is already being edited here. Add the missing entry.
7 skills/filing-jira-tickets/SKILL.md Optional. No examples/ directory; both siblings have one, and the body contains no concrete payload — no fields object, no Gherkin string, no dry-run output. At 976 words it sits just under the 1,000-word guidance floor. Leanness is a virtue, so this is a judgement call, not a defect. If added: examples/filing_workflow.md contrasting a criteria-field project (PM/SM) with one without (QA/VULN), plus a Blocks link and its verification line. Also cosmetic: siblings nest ### Step N under ## Workflow; this file uses top-level ## Step N.

Pre-existing (base branch / main) — out of scope, track separately

Location Finding
plugins/bitwarden-atlassian-tools/README.md:182 Says "Node.js 22+"; mcp/.../package.json:38 declares "node": ">=24.0.0" and repo .nvmrc is 24. Installing under Node 22 emits WARN Unsupported engine. One-word fix to a demonstrably wrong claim.
plugins/bitwarden-atlassian-tools/README.md:143-149 Documents tools as mcp__bitwarden-atlassian__<tool>; every skill in the repo uses mcp__plugin_bitwarden-atlassian-tools_bitwarden-atlassian__<tool>. The two disagree; one is wrong.
mcp/.../src/index.ts:86 Startup check !process.env[v] treats a literal unexpanded "${ATLASSIAN_CLOUD_ID}" as present, while auth.ts:12-18 resolveEnv deliberately treats it as missing. An install whose placeholders never expanded starts cleanly, then fails confusingly on first call. Fix: use resolveEnv in main().
mcp/.../src/index.ts:96 Server advertises version: "2.0.0"; package.json:3 is 1.0.0; the plugin is 2.6.0. Cosmetic, but the advertised version is what a client sees.
mcp/.../src/tools/link-issues.ts:204 Advertised inputSchema sets required: ["linkType"] only, while LinkIssuesSchema (validation.ts:312-325) is a discriminated union additionally requiring the key pair per branch. A caller can form a call the advertised schema accepts and the handler rejects. Mitigated by the "Blocks only:"/"Relates only:" property descriptions. Fix: JSON Schema oneOf with per-branch required.

Checked and cleared (not a finding)

  • bitwarden-init version in .claude-plugin/marketplace.json. Diffing this branch against main's tip appears to show a 1.2.2 → 1.2.1 downgrade. It is not one — the authoritative PR patch touches only the bitwarden-atlassian-tools line. The apparent change is branch staleness (this stack is ~165 commits behind main, predating bitwarden-init 1.2.2), and git's three-way merge keeps main's 1.2.2 because this branch never modified that line. No action needed, though rebasing the stack before merge would remove the confusion for human reviewers.
  • packageManager / devEngines.packageManager warning emitted during install: pre-existing in package.json, untouched by this PR.
  • Unconditional registration of the write tools (index.ts:39-53): deliberate and documented in a candid comment — it keeps the credential-free dry-run path available on read-only installs. Live writes are still gated in each handler. Defensible as written.

What was validated

Section Status
1. Plugin structure (plugin-validator) Run — PASS, 0 critical
2. Skill review (skill-reviewer) Run — PASS, minor items
3. Config & security (reviewing-claude-config) Run — PASS, no secrets

Manifest, marketplace consistency, version-bump policy, changelog, skill frontmatter and body accuracy, MCP server configuration and transport, credential scanning, permission scoping, and injection surface on the write path. Plus prettier, cspell, tsc --noEmit, and the full 350-test suite — all green.

Recommendation: APPROVE. The new skill is factually accurate against its tool surface, and its safety posture — dry-run default, separate opt-in write token, approval before every create, write tools deliberately left out of pre-approval — is stronger than the repo baseline. Findings 1–3 are the ones worth acting on before merge; the rest are optional.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the new filing-jira-tickets skill plus the accompanying 2.5.0 → 2.6.0 version bump across marketplace.json, plugin.json, the root README catalog, the plugin README, and the changelog. Cross-checked every behavioral claim in the skill against the write tools it drives on the base branch: the dryRun: true defaults in both CreateIssueSchema and LinkIssuesSchema, the role-named blockerKey/blockedKey direction mapping in resolveLinkDirection, link_issues's own read-back verification, get_create_fields's 404-is-ambiguous behavior, and buildDescriptionAdf's one-paragraph-per-entry construction with no line-break node — all accurate as written. No secrets, no permission-scoping concerns, frontmatter valid, and the earlier allowed-tools discussion is settled and not re-raised.

Code Review Details
  • 🎨 : Acceptance-criteria guidance states any project's criteria field is plain text, a property verified only for PM's customfield_10192; Step 1 already reports the field's schema type
    • plugins/bitwarden-atlassian-tools/skills/filing-jira-tickets/SKILL.md:33

Also noted, no change requested: the skill lists PLT among projects lacking an Acceptance criteria field, while the plugin README and the MCP source comments scope that list to QA and VULN only.

@SaintPatrck
SaintPatrck marked this pull request as ready for review August 4, 2026 20:12
@SaintPatrck
SaintPatrck requested a review from a team as a code owner August 4, 2026 20:12

- **Title.** Imperative verb, outcome, and area: `Add CSV export to the item list (web)`. Match the style of sibling tickets under the same parent, and check one if you are unsure.
- **Description.** One short paragraph of the actual work, plus any caveat specific to this ticket. No lineage boilerplate such as `Part of PM-1234`, and no path to a source document; the parent link already conveys that. Spell out shorthand rather than using symbols.
- **Acceptance criteria.** If Step 1 showed the project has a criteria field, pass the criteria there through `fields`, keyed by that field's id, as a single string of Gherkin (`Scenario`, `Given`, `When`, `Then`, `And`) — that field is plain text, so embedded line breaks render fine. If the project has no such field, put the criteria in the description instead: pass each Gherkin line as its own `descriptionParagraphs` entry, since each entry becomes one paragraph and a paragraph has no line-break node of its own. Tell the user that is what you did. Do not invent a field id.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎨 SUGGESTED: "that field is plain text" is only verified for PM's customfield_10192, yet it is stated for any project's criteria field.

Details and fix

adf-build.ts scopes the claim narrowly: "the PM project exposes Acceptance criteria (customfield_10192) as a plain textarea, so Gherkin is sent as a string and needs no ADF at all." If another project's criteria field is a rich-text field, a bare Gherkin string is the wrong payload type and the create fails.

get_create_fields already renders type <schema.type> for every field, so Step 1 has the answer. Suggested tightening:

If Step 1 showed the project has a criteria field, check the type it reported. For a string or text field, pass the criteria through fields keyed by that field's id as a single string of Gherkin (Scenario, Given, When, Then, And); line breaks render fine there. If it reports a rich-text (doc) type, a bare string will be rejected — put the criteria in the description instead and tell the user.

Rationale: everywhere else this skill refuses to assume project shape ("Never assume a field id, a required field, or that an issue type exists"), and Step 1 is already the authority on the type. This closes the one place where a shape is assumed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Speculative failure. If/when this manifests as an issue it will be addressed at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant