Skip to content

Test engineer plugin current coverage - #152

Closed
nthompson-bitwarden wants to merge 0 commit into
mainfrom
test-engineer-plugin-current-coverage
Closed

Test engineer plugin current coverage#152
nthompson-bitwarden wants to merge 0 commit into
mainfrom
test-engineer-plugin-current-coverage

Conversation

@nthompson-bitwarden

Copy link
Copy Markdown

🎟️ Tracking

https://bitwarden.atlassian.net/browse/QA-1983

📔 Objective

Slimmed down version of #150 containing only files necessary to get a "current test coverage" report

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

Plugin Validation Summary — PR #152

Verdict: ❌ FAIL — 3 critical marketplace inconsistencies must be fixed before merge.

What this PR does

PR #152 is primarily a revert/removal PR plus one new plugin:

  • Removes the entire bitwarden-ai-telemetry plugin (all files deleted).
  • Reverts bitwarden-atlassian-tools from 2.3.02.2.8, removing the three Jira Agile tools (list_boards, get_sprints, get_sprint_issues), the shared format-issue.ts helper, and their tests. search-issues.ts re-inlines its own formatIssue.
  • Reverts bitwarden-delivery-tools from 2.1.02.0.0, removing the force-multiplier skill and its evals/examples/references.
  • Adds the new bitwarden-test-toolkit plugin (assessing-test-coverage skill + references + evals).

Because the deletions are complete at the file level, the plugin-validator/skill-reviewer agents correctly reported the deleted bitwarden-ai-telemetry and force-multiplier paths as "missing" — that is the expected post-deletion state, not a defect in those deletions.

The real defects are all in .claude-plugin/marketplace.json: the version rollbacks and the plugin deletion were not propagated to the marketplace manifest.


🔴 Critical (errors — must fix)

All three live in .claude-plugin/marketplace.json and will cause ./scripts/validate-marketplace.sh (CI lint.yml) to fail.

C1 — Dangling bitwarden-ai-telemetry marketplace entry

  • File: .claude-plugin/marketplace.json lines 96–100
  • Problem: The PR deletes the entire plugins/bitwarden-ai-telemetry/ directory, but the marketplace entry remains with "source": "./plugins/bitwarden-ai-telemetry" (version 1.0.0). The source path no longer resolves — a broken marketplace reference.
  • Fix: Remove the bitwarden-ai-telemetry object (lines 96–101, including comma handling so the JSON stays valid). Confirm the metadata.version bump (now 1.1.0, line 9) still reflects the intended change.

C2 — bitwarden-atlassian-tools version mismatch

  • Files:
    • .claude-plugin/marketplace.json:51"version": "2.3.0"
    • plugins/bitwarden-atlassian-tools/.claude-plugin/plugin.json:3"version": "2.2.8"
    • plugins/bitwarden-atlassian-tools/CHANGELOG.md → latest entry ## [2.2.8] - 2026-07-01 (the 2.3.0 entry was removed by this PR)
  • Problem: The PR rolled plugin.json and CHANGELOG.md back to 2.2.8 but left marketplace.json at 2.3.0. The three sources of truth disagree.
  • Fix: Set marketplace.json:51 to "2.2.8" to match the reverted plugin.

C3 — bitwarden-delivery-tools version mismatch

  • Files:
    • .claude-plugin/marketplace.json:81"version": "2.1.0"
    • plugins/bitwarden-delivery-tools/.claude-plugin/plugin.json:3"version": "2.0.0"
    • plugins/bitwarden-delivery-tools/CHANGELOG.md → latest entry ## [2.0.0] - 2026-06-19 (the 2.1.0 entry was removed by this PR)
  • Problem: Same root cause as C2 — plugin.json/CHANGELOG.md reverted to 2.0.0, but marketplace.json still says 2.1.0.
  • Fix: Set marketplace.json:81 to "2.0.0" to match the reverted plugin.

Note on semver approach: removing tools/skills is normally a change requiring a forward version bump (per .claude/CLAUDE.md). This PR instead rolls back to the previously released versions. That is acceptable only if 2.3.0/2.1.0 were never actually published; otherwise the correct action is a new forward bump with a "Removed" changelog entry. Either way, the marketplace manifest must agree with plugin.json + CHANGELOG.md.


🟡 Minor (warnings — should fix / verify)

W1 — Atlassian MCP server package.json version frozen (pre-existing)

  • File: plugins/bitwarden-atlassian-tools/mcp/bitwarden-atlassian-mcp-server/package.json:3"version": "1.0.0"
  • The bundled server's package version does not track the plugin version. Not enforced by validation scripts; recommend syncing or documenting the decoupling.

W2 — Attachment URL scheme not explicitly restricted to HTTPS (pre-existing, low)

  • File: plugins/bitwarden-atlassian-tools/mcp/bitwarden-atlassian-mcp-server/src/utils/validation.ts (DownloadAttachmentSchema, ~lines 130–153)
  • Validates hostname/path but does not reject http://. Not exploitableclient.ts discards the caller's scheme/host and re-routes through the HTTPS gateway (https://api.atlassian.com/...). Defense-in-depth only: add a .refine() asserting protocol === "https:".

W3 — Stale default model in test-toolkit eval harness

  • File: plugins/bitwarden-test-toolkit/skills/assessing-test-coverage/evals/run_real_eval.py:173 → default model claude-opus-4-7
  • Non-blocking test-harness default (overridable via --model). Update to a current model id opportunistically.

W4 — Skill polish suggestions (both optional)

  • assessing-test-coverage/SKILL.md: no examples/ directory — a worked input→report example would strengthen grounding. Description (~900 chars) slightly exceeds the ~500-char guideline, but the overflow is a high-value explicit out-of-scope clause; keep it.

✅ Passing checks

  • bitwarden-test-toolkit (new plugin): PASS. Manifest valid; version consistent at 1.0.0 across plugin.json, CHANGELOG.md, and marketplace.json:105; assessing-test-coverage skill has valid frontmatter, exemplary trigger description with explicit anti-scope, correct progressive disclosure, and all four referenced files exist. CHANGELOG follows Keep a Changelog.
  • assessing-test-coverage skill review: PASS (only the optional W4 suggestions).
  • Deletions are clean: bitwarden-ai-telemetry and force-multiplier directories are fully removed (no orphaned component files); the only cleanup outstanding is the marketplace entry C1.
  • search-issues.ts change is benign: re-inlines formatIssue after the shared format-issue.ts helper was removed in the revert; uses the existing extractPlainTextTruncated. No behavioral or security regression.
  • Security scan: clean. No hardcoded credentials/secrets/API keys in any changed file. Atlassian secrets remain env-injected via ${ATLASSIAN_*}; the only credential-pattern matches are benign test dummies and skill-name "token" strings. HTTPS is enforced on all live Atlassian requests. No dangerous auto-approvals or overly broad file access in the changed plugin configs.

Required actions before merge

  1. .claude-plugin/marketplace.json: remove the bitwarden-ai-telemetry entry (C1), set bitwarden-atlassian-tools2.2.8 (C2), and set bitwarden-delivery-tools2.0.0 (C3).
  2. Re-run ./scripts/validate-marketplace.sh and pnpm run lint to confirm CI passes.
  3. (Optional) address W1–W4.

@theMickster theMickster left a comment

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.

A few things to change, all feel minor other that how we approach Tech Breakdowns. Great start @nthompson-bitwarden!

Comment thread plugins/bitwarden-test-engineer/CHANGELOG.md Outdated
Comment thread plugins/bitwarden-test-toolkit/README.md Outdated
Comment thread plugins/bitwarden-test-engineer/README.md Outdated
Comment thread plugins/bitwarden-test-toolkit/README.md Outdated
Comment thread plugins/bitwarden-test-engineer/README.md Outdated
Comment thread plugins/bitwarden-test-engineer/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread plugins/bitwarden-test-engineer/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread plugins/bitwarden-test-toolkit/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread plugins/bitwarden-test-toolkit/README.md Outdated

@SaintPatrck SaintPatrck left a comment

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.

Great start! In addition to the inline comments, I think it would be beneficial for everyone if we go ahead and introduce trigger evals, at a minimum, so we can objectively measure effectiveness of these additions and future changes. #126 is an example of basic evals for a design based skill. The creating-pull-requests skill in delivery-tools plugin also has some basic evals you can use as a reference. Happy to sync up outside of this PR and talk through setting them up using /skill-creator if you'd like.

Comment thread .claude-plugin/marketplace.json Outdated
Comment thread README.md Outdated
Comment thread .claude-plugin/marketplace.json Outdated
Comment thread plugins/bitwarden-test-engineer/.claude-plugin/plugin.json Outdated
Comment thread plugins/bitwarden-test-engineer/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread plugins/bitwarden-test-toolkit/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread plugins/bitwarden-test-engineer/skills/assessing-test-coverage/SKILL.md Outdated
Comment thread .cspell.json
@nthompson-bitwarden
nthompson-bitwarden marked this pull request as ready for review July 10, 2026 12:35
@nthompson-bitwarden
nthompson-bitwarden requested a review from a team as a code owner July 10, 2026 12:35
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds a new bitwarden-test-toolkit plugin with a single assessing-test-coverage skill — a backward-looking inventory of what a change is already tested by. The change is almost entirely markdown (SKILL.md, references, README, CHANGELOG) plus a Python trigger-eval runner; there is no plugin runtime code. Version and changelog requirements are met, the skill's allowed-tools are tightly scoped to read-only gh/git verbs, and it carries an explicit CWE-1427 prompt-injection boundary treating all ingested Jira/PR/CSV content as untrusted data.

Code Review Details

No blocking findings.

Validation notes (all cleared, no action needed):

  • The 6 new .cspell.json entries (mockall, Robolectric, stylesheet, Testmo, unfound, unlinkable) are each used in the added content — the earlier "dead entries" thread referenced the larger PR 150, not this slimmed version.
  • argument-hint and allowed-tools on the SKILL.md frontmatter match established conventions used by other skills in this repo.
  • The run_real_eval.py timeout loop, poll() short-circuit, and finally cleanup are correct; it is eval-only tooling.
  • Manifest versions and descriptions are consistent across marketplace.json, plugin.json, and the README table.

Note: existing reviewer threads are still being actively addressed by the author; this automated pass adds no new inline comments.

@theMickster

Copy link
Copy Markdown
Contributor

@nthompson-bitwarden Looks like ya got a quick merge conflict to resolve

@SaintPatrck

Copy link
Copy Markdown
Contributor

That merge did not go well. 😆

@nthompson-bitwarden

Copy link
Copy Markdown
Author

That merge did not go well. 😆

working on it, the pnpm check left me standed (but did it's job!) 😮‍💨

@nthompson-bitwarden
nthompson-bitwarden force-pushed the test-engineer-plugin-current-coverage branch from 4f37982 to e275d10 Compare July 22, 2026 15:56
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.

4 participants