Test engineer plugin current coverage

[nthompson-bitwarden]

🎟️ 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]

Plugin Validation Summary — bitwarden-test-engineer

PR #152 · Validated with plugin-validator, skill-reviewer, and claude-config-validator.

✅ Overall: PASS — no errors, no must-fix issues

All three validation passes succeeded. The plugin is well-structured, the single skill is high quality with correct progressive disclosure, all referenced files resolve, name/version are consistent across all sources, and no secrets or unsafe permissions were found. Only optional, advisory notes remain.

---

1. Plugin Structure (plugin-validator) — PASS

Check	Result
plugin.json JSON validity & required fields (name, version, description)	✅ valid
Semantic version 1.0.0	✅ valid
Name kebab-case, matches directory	✅ bitwarden-test-engineer
Name/version consistency — plugin.json ↔ marketplace.json:97-100 ↔ CHANGELOG.md:7	✅ consistent
Auto-discovery layout (skills/<name>/SKILL.md)	✅ correct
README.md present & comprehensive	✅
CHANGELOG.md Keep-a-Changelog format	✅
No stray/unnecessary files	✅
Hardcoded credentials scan	✅ clean

No commands, agents, hooks, or self-declared MCP servers — matches the stated single-skill scope.

---

2. Skill Review (skill-reviewer) — PASS

Skill: assessing-test-coverage

• Frontmatter: name + description present and valid. allowed-tools is tightly scoped (no blanket Bash).
• Description: strong — four concrete quoted trigger phrases, third-person, explicit scope boundary ("does NOT recommend new tests").
• Content: SKILL.md is lean (~770 words); detail correctly delegated to four references/ files (~4,100 words total). Exemplary progressive disclosure — each workflow step names the reference subsection it defers to.
• Referenced files: all four references/*.md exist on disk; all inter-file links resolve. templates/breakdown.md, tasks.md, and coverage.md mentions correctly point to the external bitwarden/tech-breakdowns repo / runtime-generated output, not local files — not broken references.

Note — false positive corrected: the skill-reviewer flagged a stray </output> tag on "line 47" of SKILL.md. Direct inspection (wc -l + tail) confirms the file is 46 lines and ends cleanly with the final Principles bullet. The </output> was the Read-tool output wrapper, not file content. No fix needed.

---

3. Security Validation (claude-config-validator) — PASS

Check	Result
Committed secrets / API keys / tokens / passwords	✅ none (only benign phrase "token spend" in README/finding-coverage)
Hardcoded credentials	✅ none
Private keys / AWS keys / Bearer tokens	✅ none
Dangerous command auto-approvals	✅ none — allowed-tools Bash(...) patterns are all narrowly scoped (gh pr view:*, git -C * rev-parse:*, etc.)
Overly broad file access	✅ none — least-privilege; no Bash(*)
settings.local.json committed	✅ n/a (no settings files in this plugin)

Notable positive: the skill embeds explicit prompt-injection defense — SKILL.md:46 and references/input-sources.md:6-10 instruct treating all ingested Jira/Confluence/PR/CSV content as untrusted data and to flag instruction-like text as a CWE-1427 concern rather than acting on it.

---

Advisory (optional, no action required)

• Minor · SKILL.md:4 — allowed-tools references cross-plugin tools from bitwarden-atlassian-tools (a Skill(...) call and 5 mcp__... tools). This is intentional and documented (README "Cross-Plugin Integration" notes graceful degradation when that plugin is absent). Flagged only to make the optional dependency explicit.
• Optional · No examples/ directory. The inline JSON record in finding-coverage.md:110-140 and the template in coverage-report-template.md largely cover this; a filled-in coverage.md sample could be a future enhancement.

---

Recommendation: APPROVE. No blocking issues. Structure, manifest, skill quality, reference integrity, version consistency, and security all pass.

[theMickster]

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

[theMickster]

❓ Do we have any reason to make our Atlassian tooling required? I would really love to see our Claude Code + testing efforts driven from directions in our Jira + Testmo tickets (if possible) 🤞🏼.

[nthompson-bitwarden]

Good question - I've kept the Atlassian tooling optional, but reframed it as the recommended driver in the README, and is already steered towards that direction in the reference files.

Ideally the skill is used early on in the SDLC - which would support making atlassian tools required - but it was purposefully designed to be used at any point in the process, even far after feature work is completed.

It could be also be used for assessing test coverage for Community PRs (which supports adding verbiage to resolve the other comment re: untrusted input boundaries).

That said, I fully agree with the direction. Perhaps the additive input sources are too broad/open-ended, which leads to variance in use, which might produce suboptimal determinism in its output.

For example, if QA has already written test cases for a feature - adding those test cases to the input along with the Jira Epic, will likely influence the output compared to if the skill was run before the test cases were written. 🤔

[theMickster]

❌ This skill has been removed so we need to refactor how we tackle referencing our tech breakdowns.

@trmartin4 do you think you could give us a hand here with some wording/phrasing? We have a challenge because we have the goal of tech breakdowns in a GitHub repo, but also still have them in Confluence that our new bitwarden-test-engineer plugin is going to need to work with.

I think we also want to be careful on how much we rely upon the tech breakdowns as a source of truth. While they are a source of truth for a period of time, once Jira tickets are crafted and code is created then we need to recognize and steer Claude more towards those and less at the tech breakdown. Correct? Or am I missing something?

[SaintPatrck]

📓 I think the reference concern is moot in this PR. regardless of what the new name or entry point is for the process that generates a tech breakdown, it can be removed entirely since it's not a true invocation. It's not really necessary to elaborate on where the tech breakdown came from.

[trmartin4]

Correct on both points.

We'd want to point this skill at the template in https://github.com/bitwarden/tech-breakdowns/tree/main/templates for contents, and the corresponding breakdown documents for each team, versus the Confluence space.

To the point about timing, I agree and I think it depends on when in the SDLC this skill is intended to be used. The intent is for completed breakdowns (i.e. the code has been implemented and is now the source of truth) will be moved to /complete subfolders for each team, so you could pretty confidently add open PRs and merged breakdowns not complete into your input sources, but guide Claude to preference the implementation over the breakdown if they conflict.

[SaintPatrck]

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.

[SaintPatrck]

💭 Since this will eventually turn into a collection of SDET tools, I'm wondering if the description and plugin name should be more generic so we don't have to constantly update it when new tools or capabilities are added. bitwarden-sdet-tools, maybe?

[nthompson-bitwarden]

I like the idea of making the description/plugin name more generic. But I feel like SDET is too scoped. Software engineer should feel free to make use of the plugin as well as management, etc... 🤔

[SaintPatrck]

⛏️ This could also be more generic to avoid future maintenance.

[SaintPatrck]

⛏️ I don't think this trailer statement is necessary.

Designed to grow additional testing capabilities over time.

Suggested change

	"description": "Test engineering toolkit for Bitwarden. Inventories the test coverage a change already has — finding existing tests across the server, client, and mobile repos, bucketing each by layer (unit, integration, E2E), citing it as a stable GitHub permalink, and flagging untested behaviors as gaps in a markdown report. Designed to grow additional testing capabilities over time."
	"description": "Test engineering toolkit for Bitwarden. Inventories the test coverage a change already has — finding existing tests across the server, client, and mobile repos, bucketing each by layer (unit, integration, E2E), citing it as a stable GitHub permalink, and flagging untested behaviors as gaps in a markdown report."

[SaintPatrck]

⛏️ Same unnecessary trailer statement repeated here.

Designed to grow additional testing capabilities over time.

Suggested change

	"description": "Test engineering toolkit for Bitwarden. Inventories the test coverage a change already has — finding existing tests across the server, client, and mobile repos, bucketing each by layer (unit, integration, E2E), citing it as a stable GitHub permalink, and flagging untested behaviors as gaps in a markdown report. Designed to grow additional testing capabilities over time.",
	"description": "Test engineering toolkit for Bitwarden. Inventories the test coverage a change already has — finding existing tests across the server, client, and mobile repos, bucketing each by layer (unit, integration, E2E), citing it as a stable GitHub permalink, and flagging untested behaviors as gaps in a markdown report.",

[SaintPatrck]

🎨

"citing each covering test as a GitHub permalink bucketed by test layer, and flagging behaviors with no observed test as gaps"

This is describing output, not when the skill should be used. Unless it was added to cover missed triggers you encountered, consider stripping it.

Trigger evals would help determine if it is adding value or not. Consider having /skill-creator build trigger evals so we can measure effectiveness now and going forward.

[SaintPatrck]

🎨 Can use arguments and argument-hint to provide cues for users about what they can pass in when invoking the skill.

It also allows reference within the skill using $ARGUMENTS[i] or $argName.

See https://code.claude.com/docs/en/skills#frontmatter-reference for details on arguments and argument-hint usage.

[SaintPatrck]

💭 This can result in files saved to different locations, depending on where the Claude session is started and its CWD. What do you think about having a single, consistent output location that doesn't run the risk of being picked up by Git? $CLAUDE_PLUGIN_DATA points to the plugin installation directory (~/.claude/plugin-data/<plugin-name>/), so maybe something like ${CLAUDE_PLUGIN_DATA}/<slug>-<date>-coverage.md? Thoughts?

[SaintPatrck]

❓ Where does the assertion that "skills can't read the clock" come from? I use date format in slugs for generated files and regularly see Claude add date and time stamps without issue. My gut says this is not accurate and should be removed, but worth double checking.

[SaintPatrck]

🎨 This is re-iterating the steps defined in researching-jira-issues, and we already noted that the Skill and MCP server are bundled together. What do you think about rephrasing it to something along the lines of...

### Epic intake

When `issuetype` is `Epic` or `Feature`, the testable behaviors live on the children and their PRs, not on the epic body. Run `Skill(bitwarden-atlassian-tools:researching-jira-issues)` on the epic key; it does child discovery, depth-controlled fan-out, and PR-link traversal in one read. The direct MCP calls (`get_issue` per child, `get_issue_remote_links` for PRs) serve targeted lookups when you do not need the full synthesized read.

Carry three things forward that are specific to this analysis:

- **Source key per behavior.** A behavior from a child links to that child, not the epic.
- **PRs are the coverage backbone.** Each child's linked PRs carry the tests that shipped, permalink-ready via the head SHA. If `gh` cannot reach one, record it as evidence-not-inspected rather than dropping it.
- **Epic status bounds expectations.** `Done` children likely have tests-in-PR to audit; `To Do` children are scope-only. Note it in Evidence.

[theMickster]

♻️ A local multi-agent code review raised the following finding.

16 of 21 added cspell words are dead entries — present in no shipped file

.cspell.json
Caught by: Code quality agent / Architecture agent
Surfaced by: both (opus + sonnet)
cmon-bro verdict: CONFIRMED — 5 words present in new files, 16 with 0 hits in new files and 0 on main

Details

This PR adds 21 words to the repo-wide cspell dictionary; only 5 (mockall, Robolectric, stylesheet, unfound, unlinkable) appear in any shipped file. The other 16 appear in no shipped file and — verified by whole-word grep across main — in no pre-existing repo file either:

actioned, automatable, Consolas, detekt, getline, inlines, ktlint, Menlo, nextest, SDET, Segoe, subdirs, tablist, tabpanel, tnum, XCUI.

These are residue from the larger PR #150 (Android/iOS/Rust tooling and UI/CSS terms from dropped content); the dictionary was not slimmed alongside the files. Repo CLAUDE.md guidance is to add domain terms only when they trip cspell on shipped content. Dead entries pollute the shared allowlist and silently accept future misspellings that collide with these tokens.

Suggested fix: drop the 16 unused words; keep only the 5 the shipped files use.