Skip to content

Feat/doc parity plugin - #193

Open
MGibson1 wants to merge 10 commits into
mainfrom
feat/doc-currency-plugin
Open

Feat/doc parity plugin#193
MGibson1 wants to merge 10 commits into
mainfrom
feat/doc-currency-plugin

Conversation

@MGibson1

@MGibson1 MGibson1 commented Aug 11, 2026

Copy link
Copy Markdown
Member

📔 Objective

Introduces the bitwarden-doc-parity plugin: a SessionStart instruction fragment, a Stop-hook tripwire for stale in-repo docs, the verifying-doc-parity skill for per-scope update-or-attest verification, and a review-context face for out-of-repo discovery. Design notes in plugins/bitwarden-doc-parity/README.md.

Evals provided with 10 cases, 3 iterations per case per configuration ~99% ± 5% with skill vs 81% ± 26% without. These pin bitwarden/server to d6c84a7562; see evals/README.md for rationale.

We will need a campaign to install this plugin on development machines as well as a follow up to enable this plugin as a feature of our review agent.

Evals against this repo were looked into, but it lacks the interconnected structure needed to exercise the skill well.
The only viable alternative to an external reference is to commit a codebase here explicitly for the purpose of evaluation, which is overkill for now and well duplicated by specifying a commit hash to reference.
currency -> parity is more common language
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown

Claude Configuration Validation — PR #193

New plugin bitwarden-doc-parity (v1.0.0): SessionStart instruction fragment, Stop-hook tripwire, verifying-doc-parity skill, and behavior evals.

Result: PASS with findings. No secrets, no malformed component definitions, and the hooks behave as documented when exercised end-to-end. One finding will fail CI as-is; four more are correctness/accuracy issues worth fixing before merge.

Severity Count
Critical 0
Major 5
Minor 8

Checks run: plugin-validator agent (structure, manifest, hooks, marketplace/version consistency), skill-reviewer agent (SKILL.md frontmatter, description, progressive disclosure), reviewing-claude-config security/structure review, shellcheck + bash -n, prettier 3.8-equivalent --check, cspell 10.0.0 with the repo config, and live functional tests of both hooks against scratch git repositories.


Major (errors — should be fixed before merge)

M1. cspell fails on the eval baseline — pnpm run lint is red

plugins/bitwarden-doc-parity/skills/verifying-doc-parity/evals/behavior-baseline.json:452, :1455, :1488, :3692

Reproduced with the CI cspell version (10.0.0) and the repo's .cspell.json:

behavior-baseline.json:452:97   - Unknown word (mangl)
behavior-baseline.json:452:187  - Unknown word (mangl)
behavior-baseline.json:1455:106 - Unknown word (retargeted)
behavior-baseline.json:1488:127 - Unknown word (retargeted)
behavior-baseline.json:3692:25  - Unknown word (Colocating)

The file is neither gitignored nor in .cspell.json ignorePaths, so lint:spelling exits 1 and the lint.yml workflow fails. All five hits are model-generated evidence prose in recorded baseline data.

Fix: add mangl, retargeted, and Colocating to the words array in .cspell.json (alphabetized), or // cspell:ignore-style suppression is not available in JSON — the dictionary is the practical route here.

M2. The plugin's normative reference URL returns 404, and it is injected into every session

  • plugins/bitwarden-doc-parity/hooks/doc-parity-instructions.md:5 (SessionStart context — every session in every consuming repo)
  • plugins/bitwarden-doc-parity/skills/verifying-doc-parity/SKILL.md:12 and :39
  • plugins/bitwarden-doc-parity/README.md:3

Verified directly: https://contributing.bitwarden.com/contributing/documentationHTTP 404 (absent from the site's sitemap). The related ADR is live and returns 200/308: https://contributing.bitwarden.com/architecture/adr/adopt-engineering-documentation-standard, but its status is PROPOSED (published 2026-08-05) and it states only that the standard "will be published as the Documentation section under Contributing".

This is load-bearing rather than cosmetic: SKILL.md:39 instructs the agent to "Consult the standard" for placement/format/style questions, SKILL.md:69 defers the entire external-docs flow to it, and the skill's allowed-tools grant exists specifically to fetch that domain. A run following the instruction fetches a 404 with no fallback.

Fix: point all four references at the live ADR URL until the standard page ships (and re-point in a follow-up), or land the contributing-docs page before merging this PR.

M3. README asserts an integration with a bitwarden-code-review version that does not exist

plugins/bitwarden-doc-parity/README.md:31 — "When installed alongside bitwarden-code-review (from 1.14.0), the reviewer agent's Cross-Plugin Enrichment picks the skill up automatically."

In this repo bitwarden-code-review is at 1.13.1 (.claude-plugin/marketplace.json, its plugin.json, and the root README catalog all agree), and its Cross-Plugin Enrichment section (plugins/bitwarden-code-review/agents/bitwarden-code-reviewer/AGENT.md:65-85) does not reference verifying-doc-parity. As shipped, Layer 4 has no activation path in this marketplace.

Fix: reword to state that the enrichment wiring lands in a follow-up code-review release, or include the code-review 1.14.0 change in this PR.

M4. "Documented scope" is defined three different ways, and the hook comment claims an alignment that isn't there

  • plugins/bitwarden-doc-parity/skills/verifying-doc-parity/SKILL.md:31 — "a README.md, a docs/ directory, or diagram sources" (no CLAUDE.md)
  • plugins/bitwarden-doc-parity/hooks/doc-parity-check.sh:120 — also matches CLAUDE.md, with the comment on :113-114 asserting this is "aligning with SKILL.md's scope definition" (it is not)
  • plugins/bitwarden-doc-parity/hooks/doc-parity-check.sh:5-8 — the script's own header comment says a documented scope is "a README.md or a docs/ directory", contradicting its own line 120
  • evals/behavior-eval.json cases 2 and 10 expect util/Seeder/CLAUDE.md to be read and attested

Tiebreaker: ADR-0034 states verbatim that "AI instruction files are explicitly not documentation and are out of the standard's scope." That supports SKILL.md's definition and argues for removing CLAUDE.md from the hook's marker set and from the eval expectations — but whichever way it is resolved, the four artifacts must agree.

Fix: pick one definition; update doc-parity-check.sh:5-8, :113-114, :120, SKILL.md:31, and eval cases 2/10 to match.

M5. Three graded eval expectations are not derivable from the skill text

  • External-docs flowSKILL.md:69 says only "triggers the standard's external-docs flow per the documentation standard", but eval case 10 grades the run on stating "a work item created before merge and a stale marker applied to the page". That text lives in README.md:29 (never loaded into skill context) and in the 404'd standard.
  • Anti-token-edit rule — eval case 3 grades "Refuses to make the requested cosmetic or token documentation edit", but that rule appears only in the Stop hook's block message (doc-parity-check.sh:149). On the on-demand and PR-review paths the hook message is absent, so the rule is missing entirely.
  • Review-context UpdateSKILL.md:39 unconditionally says "Edit the documentation in the same change", with no branch for the PR-review context established at :19, where a reviewer cannot edit the PR. Only the out-of-repo case is carved out.

Fix: inline the work-item + stale-marker actions at SKILL.md:69; add the anti-token-edit rule to Step 3; add one sentence stating that in review context an Update becomes a review finding naming the file and required change.


Minor (warnings)

m1. Diagram sources other than Mermaid are classified as code

plugins/bitwarden-doc-parity/hooks/doc-parity-check.sh:65-71is_doc_file recognizes .md, .mdx, .mmd, .mermaid, and anything under docs/. The plugin description and SKILL.md:31 both treat diagrams as documentation, so a .puml, .drawio, or .excalidraw update outside a docs/ directory earns no coverage credit and is itself counted as a changed code file — a false-positive block. Impact is capped (one block per session, and Step 1 of the skill has a dismissal path). Fix: extend the is_doc_file case list with the diagram extensions the standard recognizes.

m2. compgen -G treats [ in a directory name as a glob class

doc-parity-check.sh:119 — a path such as src/my[comp]/ with a README present is not detected as a documented scope. Fails open (a missed block, never a false block). Paths with spaces, *, ?, and non-ASCII characters were tested and work correctly. Fix: [[ -n $(find "$REPO_ROOT/$dir" -maxdepth 1 -iname 'readme.md' -print -quit 2>/dev/null) ]].

m3. Stop-hook timeout headroom is thin on large working trees

hooks/hooks.json:20 sets "timeout": 15; the validator measured ~8.9s on a 2,000-file working tree. documented_ancestors (doc-parity-check.sh:115-125) re-walks and re-stats each directory once per changed file, so a large refactor on slower hardware can exceed the budget — in which case the hook is killed and silently never fires. Fix: memoize the per-directory lookup, and/or raise the timeout to 30.

m4. Marker files accumulate, and the marker directory is unhardened against /tmp symlinks

doc-parity-check.sh:30-43${TMPDIR:-/tmp}/doc-parity/blocked-<session_id> is never pruned (one file per blocking session). On a shared host, a pre-existing attacker-owned /tmp/doc-parity symlink would redirect the touch. Real impact is limited to creating or touching an empty file, and the -O ownership check at :40 already prevents block suppression, so this is hardening only. Fix: optionally prune markers older than a day on entry, and verify $MARKER_DIR is a real directory ([[ -d "$MARKER_DIR" && ! -L "$MARKER_DIR" ]]) before use.

m5. allowed-tools does not cover the tools the out-of-repo step needs

SKILL.md:7 grants only WebFetch(domain:contributing.bitwarden.com), while SKILL.md:67 says to search contributing-docs and names the bitwarden/contributing-docs source repo. allowed-tools on a skill is an additive permission grant rather than a restrictive allowlist (confirmed against the installed Claude Code frontmatter schema), so Read/Bash/Edit still work and this is not a functional break — but in a headless CI review an un-preapproved WebSearch or raw.githubusercontent.com fetch can be denied with no human to prompt. Fix: add WebSearch and WebFetch(domain:raw.githubusercontent.com), or drop the source-repo mention.

m6. The coverage rule is broader than the header comment states

doc-parity-check.sh:101-110is_covered accepts any touched doc whose scope is an ancestor prefix of the changed file, including a doc strictly deeper than the documented ancestor that armed the tripwire. Changing a/b/code.cs where a/README.md is the documented scope is suppressed by touching an unrelated a/b/notes.md. This looks deliberate (the semantic layer catches the rest), but the header comment at :5-13 documents only the trigger rule. Fix: document the coverage rule next to the trigger rule.

m7. CHANGELOG release date predates the work

plugins/bitwarden-doc-parity/CHANGELOG.md:8## [1.0.0] - 2026-08-04, while the plugin's first commit is 2026-08-05 and the PR is unmerged as of 2026-08-11. Fix: set the date to the expected merge date.

m8. SKILL.md prose nits

  • :12 "This skill is the judgment that code has drifted away from the documentation which describes it" reads as asserting drift; prefer "judges whether code has drifted from the documentation that describes it".
  • :27 uses bold lowercase dismiss while :56 uses "Dismiss" and :39/:40 use Update. / Attest. — make the three outcome names consistently capitalized, since the evals treat Dismiss as a first-class outcome.
  • :39 "isn't obvious from what you already have" is the file's only second-person phrasing; rewrite in imperative form.
  • :31 relies on "diagram sources" without linking the diagram standard, which is live at https://contributing.bitwarden.com/contributing/diagrams — worth linking given M2.
  • README.md:49-51 Requirements lists bash, git, jq but not a minimum Claude Code version; per the validator's reading of the skills reference, background: false requires Claude Code v2.1.218+. Worth stating for consumers.

Verified clean

Structure and manifest

  • .claude-plugin/plugin.json is valid JSON; name is kebab-case and matches the directory; version is semver 1.0.0; field set (name, version, description, author, homepage, repository, keywords) matches the convention used by the other 16 plugins.
  • Version is consistent across plugin.json, .claude-plugin/marketplace.json, and the root README.md:18 catalog row. No agents exist, so no AGENT.md versions to sync. CHANGELOG present, Keep a Changelog format, dated [1.0.0] heading with an Added section.
  • Auto-discovery paths are correct (hooks/hooks.json, skills/*/SKILL.md); no explicit manifest paths needed, matching bitwarden-ai-telemetry.
  • Components: 0 commands, 0 agents, 0 MCP servers, 1 skill, 2 hooks.

Hooks

  • hooks.json uses real event names (SessionStart, Stop), invokes both scripts as bash "${CLAUDE_PLUGIN_ROOT}/hooks/<script>.sh", and sets timeouts.
  • Functional tests in scratch repos confirm: blocks with well-formed {"decision":"block","reason":…} when code changes inside a documented scope with no doc touched; silent on the second call in the same session (marker); silent when stop_hook_active is true; silent on doc-only changes; silent when a covering doc is touched. SessionStart emits correct hookSpecificOutput / hookEventName / additionalContext, and the CLAUDE_PLUGIN_ROOT fallback in doc-parity-context.sh:13 works when the variable is unset.
  • Fail-open is real: no set -e, guarded early exits for missing jq, missing cwd, non-git directories, and empty change sets; all git calls use || true / 2>/dev/null.

Security

  • No secrets, API keys, tokens, or passwords in any plugin file. (Every token match is the phrase "token documentation edit".)
  • No settings.json / settings.local.json changes; no permission or auto-approval changes; no dangerous command allowlisting.
  • Shell safety: no eval, all expansions quoted, session_id sanitized with tr -cd 'a-zA-Z0-9_-' (blocks path traversal in the marker filename), printf format strings are literals with data passed as arguments, and the block reason is emitted via jq -n --arg. core.quotePath=false handles non-ASCII paths. shellcheck is clean on both scripts; bash -n passes. Both scripts are 0755.
  • The skill's WebFetch grant is domain-scoped rather than open — good least-privilege practice.

Skill

  • Frontmatter is valid, including the less common agent / context: fork / background: false trio (checked against the installed Claude Code frontmatter schema); name matches the directory.
  • Description is 560 chars, third-person, with six concrete quoted trigger phrasings plus both machine invocation paths — the strongest description in the repo.
  • Body is 792 words: below the 1,000–3,000 guidance band but with no bloat; progressive disclosure is appropriate as-is (no references/ warranted at this size, and the two report examples define the output contract so they belong in the core file). evals/ is correctly not referenced from SKILL.md, so it costs no context.
  • Internal anchor #out-of-repo-discovery-review-context-only (:19) resolves to the heading at :62; README relative links ./hooks/doc-parity-instructions.md and ./hooks/doc-parity-check.sh resolve.
  • Evals are well-formed: 10 cases, baseline with 100 runs (10 cases × 2 configurations × 5 iterations), exceeding the ≥3-iteration guidance, with a documented regression-diff procedure.

Formatting

  • prettier --check passes on every markdown and JSON file in the plugin plus .claude-plugin/marketplace.json and the root README.md. All JSON files parse.

Recommended merge order

  1. Add the three words to .cspell.json (M1) — this alone blocks CI.
  2. Repoint the documentation-standard URL in all four locations (M2).
  3. Correct or soften the bitwarden-code-review 1.14.0 claim (M3).
  4. Reconcile the CLAUDE.md scope definition across hook, skill, and evals (M4), and add the missing skill rules from M5.
  5. Sweep the minors — the compgen/memoization pair (m2, m3) can be fixed in one edit.

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Re-reviewed bitwarden-doc-parity after the four follow-up commits. The earlier eval findings are addressed: behavior-baseline.json now records runs_per_configuration: 5 with real executor/analyzer models (matching the 100 runs it contains), and the evals README's regression check is scoped to a jq projection over run_summary and per-run pass rates, so an empty diff is now reachable. The Stop-hook hardening (plugin-owned marker directory with an -O ownership check, core.quotePath=false on both git queries, case-insensitive README matching) is sound and still fails open on missing jq, missing cwd, or a non-git working directory. Remaining findings are consistency gaps between the tripwire, the skill, and the plugin README.

Code Review Details
  • ♻️ : Hook arms on CLAUDE.md as a documented-scope marker, but SKILL.md Step 2 does not list it, so a block can name a scope the skill will not enumerate; file-header trigger rule is stale
    • plugins/bitwarden-doc-parity/hooks/doc-parity-check.sh:112-120
  • ♻️ : README documents automatic reviewer pickup "from 1.14.0" of bitwarden-code-review, which is at 1.13.1 with no reference to verifying-doc-parity
    • plugins/bitwarden-doc-parity/README.md:31

One earlier thread on SKILL.md allowed-tools scope for the forked agent is still open; not re-raised here.

Comment thread plugins/bitwarden-doc-parity/skills/verifying-doc-parity/SKILL.md Outdated
description: Use this skill whenever the user mentions doc drift, documentation verification, README or docs/ updates that follow a code change, or a doc-parity Stop hook block — even if the request does not name a skill or documentation explicitly. Also use as the documentation pass of a pull request review. Verifies or updates documentation at every documented ancestor scope of a code change; in review context, also discovers out-of-repo documentation the change invalidates. Triggered by phrasings such as "verify doc parity", "are my docs up to date", "did I miss any doc updates", "what documentation should this change touch", "do I need to update any docs for my current changes", or "check if the docs still match the code".
agent: general-purpose
context: fork
allowed-tools: WebFetch(domain:contributing.bitwarden.com), WebFetch(domain:contributing-docs.bitwarden.com)

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.

QUESTION: allowed-tools lists only WebFetch — does the forked skill still get the tools its workflow needs?

Details

The workflow requires git diff HEAD / git ls-files (Step 1), directory walking and doc reads (Step 2), and doc edits (Step 3), but allowed-tools names only two WebFetch domains. Every other skill in this repo enumerates everything it uses, e.g.:

  • plugins/bitwarden-code-review/skills/performing-multi-agent-code-review/SKILL.md: Bash(git diff:*), Read, Write, Grep, Glob, ...
  • plugins/claude-config-validator/skills/reviewing-claude-config/SKILL.md: Read, Grep, Glob

If allowed-tools restricts the tool set for the forked general-purpose agent, the skill cannot assemble the change or apply the Update outcome — and a skill-creator benchmark harness would not necessarily surface that. Was this exercised in a real session (hook block → skill run → doc edit), or is allowed-tools purely additive permission granting here?

Comment thread plugins/bitwarden-doc-parity/skills/verifying-doc-parity/evals/README.md Outdated
@MGibson1 MGibson1 added the ai-review-vnext Request a Claude code review using the vNext workflow label Aug 11, 2026
@MGibson1
MGibson1 force-pushed the feat/doc-currency-plugin branch from b613324 to feb01ac Compare August 11, 2026 18:04
Comment thread plugins/bitwarden-doc-parity/skills/verifying-doc-parity/evals/README.md Outdated
Comment on lines +112 to +120
# Documented ancestor scopes of a file, strictly below the repo root.
# Match README case-insensitively; CLAUDE.md and docs/ are the other markers,
# aligning with SKILL.md's scope definition.
documented_ancestors() {
local dir
dir=$(dirname "$1")
while [[ "$dir" != "." && "$dir" != "/" ]]; do
if compgen -G "$REPO_ROOT/$dir/[Rr][Ee][Aa][Dd][Mm][Ee].md" >/dev/null 2>&1 \
|| [[ -f "$REPO_ROOT/$dir/CLAUDE.md" || -d "$REPO_ROOT/$dir/docs" ]]; then

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.

♻️ DEBT: The tripwire now arms on CLAUDE.md, but the skill it hands off to does not treat that as a documented scope.

Details and fix

The comment on line 113-114 says CLAUDE.md aligns "with SKILL.md's scope definition", but SKILL.md:31 defines a documented scope as a directory containing "a README.md, a docs/ directory, or diagram sources" — CLAUDE.md appears nowhere in SKILL.md (or in the plugin README.md).

Consequence for a directory that carries a CLAUDE.md but no README.md/docs/ (common for component-level agent guidance in monorepos): the hook blocks and names that directory under "Documented scopes involved", then Step 2 of the skill does not enumerate it, so Step 4's per-scope report cannot attest to the scope the block called out. The developer gets a gate with nothing to answer.

The reverse gap exists too: SKILL.md counts bare diagram sources as a documented scope, while documented_ancestors only recognizes README/CLAUDE.md/docs/.

Pick one definition and use it in both places — either add CLAUDE.md to SKILL.md:31, or drop it from line 120. Either way, the file-header trigger rule on lines 7-8 ("contains a README.md or a docs/ directory") is now stale and should match.


The same skill logic run as a PR reviewer through the consuming repository's ai-review workflow, covering human-authored changes that no in-session layer sees. This layer also owns out-of-repo discovery. Search terms are derived from the diff and used to search contributing-docs for references that need to be addressed. The callout triggers the standard's external-docs flow: a work item before merge and a stale marker on the page.

This layer is inert until the review pipeline invokes `verifying-doc-parity`. When installed alongside `bitwarden-code-review` (from 1.14.0), the reviewer agent's Cross-Plugin Enrichment picks the skill up automatically. If the consuming repository uses a different review workflow, wire the skill into that workflow's documentation pass.

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.

♻️ DEBT: This documents automatic reviewer pickup against a bitwarden-code-review version that does not exist yet.

Details and fix

bitwarden-code-review is at 1.13.1 in .claude-plugin/marketplace.json (this PR does not change it), and nothing in plugins/bitwarden-code-review/ references verifying-doc-parity — its Cross-Plugin Enrichment section enumerates specific skills by name, so there is no generic pickup path.

The PR description confirms the wiring is a planned follow-up, but the sentence reads as present tense for anyone installing today: "(from 1.14.0), the reviewer agent's Cross-Plugin Enrichment picks the skill up automatically." A reader installing both plugins now would assume Layer 4 is live and get no review coverage.

Either land the enrichment entry (plus version bump) in this PR, or mark the integration as planned, e.g. "Automatic pickup by the bitwarden-code-review reviewer agent is planned; until then, wire the skill into your review workflow's documentation pass."

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

Labels

ai-review-vnext Request a Claude code review using the vNext workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant