add/testing tools context and scoping

[kdenney]

🎟️ Tracking

📔 Objective

[github-actions]

Claude Code validation

Result: Issues found

Validated the bitwarden-testing-tools plugin changes in PR #210 — three new planning-phase agents, two new skills, a shared Playwright tool policy, and three known-flow catalogs. Findings are pinned to this PR's own merge base, 5703fe78 (branch add/testing-tools-pipeline-foundation), not main — see the scope note below.

Two things drive the verdict: a frontmatter YAML defect that silently truncates both new skill descriptions, and a permitted-but-unconstrained tool category in the shared policy that this same PR then exercises.

Scope note — stacked PR

This PR targets add/testing-tools-pipeline-foundation, not main. The changed-file list supplied to this review was computed against a different base, so it names ~22 files this PR does not touch. The real diff is 20 files (+1075/−9).

Confirmed unchanged by this PR, and therefore not reviewed (they arrived on the parent branch — the last commit touching them is the merge base, and their git tree objects are byte-identical at both ends): skills/reading-mailcatcher-api/**, skills/using-stripe-cli/**, skills/assessing-test-coverage/evals/**, scripts/eval_harness.py, scripts/running-evals.md. Auditing those here would attribute the parent branch's decisions to this PR. If they need review, it belongs against the parent branch's own base. Worth fixing the base used to generate the list, since every reviewer on this PR is pointed at the same inherited files.

Critical

• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:3 and plugins/bitwarden-testing-tools/skills/mapping-services-under-test/SKILL.md:3 — Skill descriptions are silently truncated by YAML comment parsing. Both are unquoted plain scalars containing # (## States, ## Flows), and in YAML a # preceded by whitespace opens a comment. Verified with a real parse: the scoping description loads 238 of 426 characters, ending mid-sentence on a dangling em dash; the mapping description loads 225 of 479. Everything after the # — the entire output contract in both cases — never reaches the model deciding whether to invoke the skill. This is new breakage, not house style: the pre-existing writing-manual-test-cases description parses at full length. Fix: quote the scalar (description: "…") or use a >- block scalar. Fix both.

Major

• plugins/bitwarden-testing-tools/references/playwright-tool-policy.md:7 — Category 3 is declared permitted but never constrained, and this PR ships a Category 3 step. Line 7 lists "External trigger simulation" as one of four permitted categories and line 10 promises "the constraints for each category present in this pipeline", but the headings jump Category 2 (line 24) → Category 4 (line 28). Category 1 is explicitly bounded to localhost, 127.0.0.1, ::1, or bitwarden.test origins (line 22); Category 3 gets no origin bound, no method bound, and no body bound. The ## Never Permitted list (lines 32–39) does not close the gap — "API calls that substitute for UI actions" excludes exactly what Category 3 covers. Meanwhile skills/scoping-playwright-test-cases/references/known-flows/billing.md:161 adds EXTERNAL TRIGGER: POST to http://localhost:33656/accounts/trial/send-verification-email with a JSON body, so Category 3 is present and line 10's promise is false. Fix: add a ## Category 3 - External Trigger Simulation section carrying at minimum Category 1's origin allowlist, and name the sanctioned methods/endpoints. This is the security-weakening finding that fails the run.

• plugins/bitwarden-testing-tools/references/playwright-tool-policy.md:1 — The tool policy is orphaned; nothing loads it. It sits at plugin root, not an auto-discovered location, and no SKILL.md, AGENT.md, or README line cites its path. The only mentions are bare prose ("the tool policy") in skills/scoping-playwright-test-cases/SKILL.md:64 and the eval files. That line is load-bearing — the [HUMAN] marker downstream consumers detect deterministically depends on knowing what the policy disallows — yet the skill never names or links the file, and it lives outside the skill's own references/, so the ${CLAUDE_SKILL_DIR} convention at line 14 cannot reach it. As shipped, the boundary is unenforced. Fix: cite ${CLAUDE_PLUGIN_ROOT}/references/playwright-tool-policy.md explicitly at SKILL.md:64, add it to the line 14 pre-read list, and list it in the plugin README.

• plugins/bitwarden-testing-tools/agents/services-under-test-mapper/AGENT.md:8 and plugins/bitwarden-testing-tools/agents/playwright-test-case-scoper/AGENT.md:8 — Bash tool grants use a space where the prefix form needs a colon. Both declare Bash(git diff *) / Bash(git log *); the documented prefix-match syntax is Bash(git diff:*). The repo's only other agent, plugins/bitwarden-code-review/agents/bitwarden-code-reviewer/AGENT.md, uses the correct colon form. This under-matches rather than over-grants, so it is a correctness defect, not a privilege escalation — but services-under-test-mapper's only Bash grant is this one, and the skill it delegates to documents git diff --name-only origin/main...HEAD -- <path>, so the agent may have no path to its required input. Fix: tools: Read, Skill, Bash(git diff:*) and tools: Read, Skill, Grep, Glob, Bash(git diff:*), Bash(git log:*).

• Four forward references to components that do not exist in the repository. This repo already treats these as defects — commit 22c883d is "drop forward reference to billing-test-data.md in scoping layer".

  Reference	Cited at	Exists
  start-playwright-test skill	all three agents/*/AGENT.md:3	No
  writing-playwright-test-cases	skills/scoping-playwright-test-cases/SKILL.md:6; references/known-flows/{admin,auth,billing}.md:3	No
  localhost-web-health-checker / health-check.sh	skills/mapping-services-under-test/SKILL.md:26	No
  playwright-cli skill	references/playwright-tool-policy.md:20, called a declared prerequisite	Not declared

  The start-playwright-test case is the sharpest: it is the entire "when to use" clause of all three agent descriptions ("Do not invoke directly; dispatched by the start-playwright-test skill"), so the only documented trigger path points at nothing. The mapping case is load-bearing differently — SKILL.md:26 rests a MUST on "health-check.sh … exits 1 on anything else", a claim no reader can verify. Fix: land the referents or reword each site generically. For playwright-cli, also correct plugins/bitwarden-testing-tools/README.md:46, which still says "Two skills need an external tool" and lists only Stripe CLI and Mailcatcher.

• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:1-4 and plugins/bitwarden-testing-tools/skills/mapping-services-under-test/SKILL.md:1-4 — Both new skills omit allowed-tools while instructing shell execution. Every pre-existing skill in this plugin constrains itself (using-stripe-cli pins a single script path), yet both new skills run git (scoping/SKILL.md:21, mapping/SKILL.md:6,15) with only name and description declared. The wrapping agents constrain tools today, but invoked directly by a user, each runs with the session's full tool set. Fix: allowed-tools: "Read, Grep, Glob, Bash(git diff:*), Bash(git log:*)" on the scoping skill; allowed-tools: "Read, Bash(git diff:*)" on the mapping skill.

• plugins/bitwarden-testing-tools/skills/mapping-services-under-test/SKILL.md:20 — The Admin primary-URL branch can name a URL for a service absent from its own output. Step 6 selects the Bitwarden Portal "when only Admin routes are present", but references/services.md:47 scopes Admin to server/src/Admin/** changes only, with no route clause — so step 3 can never add Admin. Routes on Admin pages plus a diff touching only server/src/Api/** yields a union of Api/Identity/Web while step 6 emits http://localhost:62911 as the primary test URL, for a service that is never started or health-checked. Fix: add a route clause to the Admin entry in references/services.md, or restrict step 6 to services already in the union.

• plugins/bitwarden-testing-tools/skills/mapping-services-under-test/SKILL.md:19 — The worked example for an empty union is an input the reference says cannot produce one. Step 5 cites "only clients/apps/web/** template-only changes with no routes", but references/services.md:17 makes clients/apps/web/** a Required by: trigger for Web, so that input yields {Web}. A reader applying step 2 correctly cannot tell whether the fallback or the match wins. Fix: use an input that genuinely matches nothing (e.g. changes confined to server/test/**), or drop the example.

• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:105-107 — The output schema has no slot for the script-verified state its own catalog mandates copying verbatim. references/known-flows/billing.md:103-108 defines state:trial-verification-email-received with Route: n/a, a Mailcatcher script call as its Selector, and Expectation: stdout contains a https://… — outside the documented enum <visible | hidden | disabled | text contains "..." | count = N>. It is explicitly not a [HUMAN] step (billing.md:95). Lines 14/32/68 require copying it verbatim, and terminal self-review check 4 (line 140) polices only text contains points, so it passes review and lands malformed in the artifact downstream consumers parse. Fix: add a sanctioned third verification-point form for policy-permitted non-UI script checks, with its own marker parallel to [HUMAN], to both the schema block and the self-review checks — or reshape the billing entry to fit the enum.

• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:3 — The description's trigger contradicts the skill's own input contract and the rationale its eval suite uses to omit untrusted-content handling. The description says "Use before building test cases whenever a Jira ticket or plan is provided", while line 6 states the inputs are already-extracted "affected repos, feature description, and acceptance criteria". evals/README.md leans on that contract to scope out untrusted-content handling, arguing the SKILL.md "never mentions Jira, tickets, or untrusted source content" — but the description does, and the description is the field that governs autonomous invocation. So the skill can be triggered directly on raw ticket content while carrying none of the untrusted-data framing both wrapping agents carry. Fix: restate the trigger in terms of the actual input, or accept raw-ticket entry and add the untrusted-content paragraph to SKILL.md.

Minor

• plugins/bitwarden-testing-tools/CHANGELOG.md:7 — ## [1.3.0] - 2026-08-24 sits above ## [1.2.0] - 2026-08-31, breaking Keep a Changelog's reverse-chronological ordering. Set it to the merge date.
• plugins/bitwarden-testing-tools/agents/playwright-test-case-scoper/AGENT.md:7 — color: orange is outside the accepted set (blue, cyan, green, yellow, magenta, red). The two sibling agents in this PR correctly use green and blue.
• plugins/bitwarden-testing-tools/skills/mapping-services-under-test/SKILL.md:28 — Notifications, Events, and Icons are declared valid output tokens, but their entries in references/services.md:85,95,105 use Start if: rather than Required by:, and steps 2–3 match only Required by:. Say explicitly that optional-infrastructure services are excluded from the upfront list.
• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:14 — "Read all three catalogs" hardcodes the count; a fourth catalog under references/known-flows/ would be silently skipped. Use "read every catalog under…".
• plugins/bitwarden-testing-tools/agents/services-under-test-mapper/AGENT.md:26 — blind collection of every Route: <URL> line will pick up - Route: n/a (billing.md:103) and templated routes like /organizations/:organizationId/vault (billing.md:41,60,79). Tell the agent to skip n/a and match templated routes on their static prefix.
• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/SKILL.md:114-127 — the schema shows flow fields as bare bold lines (**Use when:**); every catalog writes them as list items (- **Use when:**). "Copy verbatim" therefore yields an artifact mixing two formats. Normalize the catalogs to the schema's shape.
• Both new skill descriptions (scoping/SKILL.md:3, mapping/SKILL.md:3) carry no quoted user trigger phrases and no negative scope, unlike siblings writing-manual-test-cases and assessing-test-coverage. Adding both would also mitigate the trigger collision noted above.
• ${CLAUDE_SKILL_DIR} (scoping/SKILL.md:14, mapping/SKILL.md:6) is not a documented Claude Code variable — the documented one is ${CLAUDE_PLUGIN_ROOT}. The convention predates this PR, so this is consistency, not new breakage; but mapping/SKILL.md uses it once and a bare relative path for the same file four times (lines 16, 17, 24, 26). Pick one form.
• plugins/bitwarden-testing-tools/skills/scoping-playwright-test-cases/references/known-flows/auth.md:35,36,47 — the literal dev master password test-master-password-12. This is a localhost fixture, not a live secret, and the same PR correctly forbids resolving the admin email from server/dev/secrets.json — but a literal password string in a Bitwarden repo will trip secret scanners. Consider a placeholder resolved the same way the admin-email token is.
• The three new AGENT.md files carry no version: frontmatter, so they do not participate in bump-plugin-version.sh or the documented bump procedure. Repo practice is mixed (2 of 8 agents carry it), so nothing is out of sync today.
• plugins/bitwarden-testing-tools/agents/playwright-test-context-gatherer/AGENT.md:73 states the UNTRUSTED SOURCE CONTENT markers are decorative — "Nothing downstream parses them, and no component's behavior depends on them." The downstream scoper does carry its own untrusted-content preamble, so the boundary is enforced by prompt rather than by structure. The honesty is good practice; flagging only so the choice is a deliberate one on the record.

What passed

• Version bump: correct. 1.2.0 → 1.3.0 is consistent across .claude-plugin/marketplace.json:105, plugins/bitwarden-testing-tools/.claude-plugin/plugin.json:3, and the README.md:22 catalog row, with a matching ## [1.3.0] changelog entry covering every new component. MINOR is the right bump for backward-compatible additions.
• No prompt injection (CWE-1427). All three agent files, both new skills, the tool policy, the three catalogs, and both eval JSONs were read for text addressed to a reviewer, claimed repository policy, or attempts to steer tooling. Nothing found. All three agents in fact open with a correct untrusted-content preamble treating Jira/Confluence content as data.
• No credentials. No settings.local.json in the changeset. 4242424242424242 is Stripe's published test card; test-master-password-12 is a documented localhost fixture (noted above).
• Manifest valid, no hooks and no MCP server config in this plugin, both new behavior-eval.json files are valid JSON, and prettier --check and cspell pass on all changed files.
• Progressive disclosure is well executed in both new skills, and every file path referenced from scoping-playwright-test-cases resolves on disk.

Checks run

Check	Status
Plugin structure (script)	Skipped — run by the workflow as a separate step; see the job log and check status
Marketplace (script)	Skipped — run by the workflow as a separate step; see the job log and check status
Version bump (script)	Skipped — run by the workflow as a separate step; AI validation separately confirmed 1.2.0 → 1.3.0 consistent across all four required files
Plugin validation (AI)	Ran — issues found
Skill review (AI)	Ran — issues found. 2 of the 4 listed skills reviewed (scoping-playwright-test-cases, mapping-services-under-test); reading-mailcatcher-api and using-stripe-cli confirmed byte-identical to the merge base and correctly not audited
Configuration & security	Ran — issues found. Covered 3 agents, the tool policy, and the skill support files under references/; no CLAUDE.md, .claude/, command, hook, or settings files changed in this PR