[AI-62] feat: Add filing-jira-tickets skill to bitwarden-atlassian-tools - #189
[AI-62] feat: Add filing-jira-tickets skill to bitwarden-atlassian-tools#189SaintPatrck wants to merge 4 commits into
Conversation
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.
Claude Configuration Validation — PR #189Verdict: PASS. No critical or major issues. No blocking errors. The findings below are minor and mostly optional.
Scope note (affects how to read this report)This is a stacked PR: base is
The MCP server sources in the task's changed-file list ( Corrections to automated findingsTwo 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. Plugin Validation — PASS
Version-bump policy — PASS
CI pre-checks (run locally)
2. Skill Review — PASS (minor items)
Factual accuracy against the tool surface is excellent. Every API claim was verified in source: dry-run defaults ( 3. Configuration & Security Review — PASS
Security design highlights (worth preserving)
FindingsAll minor. None block merge. In this PR's diff
Pre-existing (base branch /
|
| 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-initversion in.claude-plugin/marketplace.json. Diffing this branch againstmain's tip appears to show a1.2.2 → 1.2.1downgrade. It is not one — the authoritative PR patch touches only thebitwarden-atlassian-toolsline. The apparent change is branch staleness (this stack is ~165 commits behindmain, predatingbitwarden-init1.2.2), and git's three-way merge keepsmain's1.2.2because this branch never modified that line. No action needed, though rebasing the stack before merge would remove the confusion for human reviewers.packageManager/devEngines.packageManagerwarning emitted during install: pre-existing inpackage.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.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the new Code Review Details
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. |
|
|
||
| - **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. |
There was a problem hiding this comment.
🎨 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
fieldskeyed 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.
There was a problem hiding this comment.
Speculative failure. If/when this manifests as an issue it will be addressed at that time.
🎟️ Tracking
AI-62
📔 Objective
Adds the
filing-jira-ticketsskill tobitwarden-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.