Skip to content

[BRE-1830] feat(bitwarden-devops-engineer): Add naming conventions - #192

Open
fntyler wants to merge 5 commits into
mainfrom
bre-1830/skill-workflow-naming-conventions
Open

[BRE-1830] feat(bitwarden-devops-engineer): Add naming conventions#192
fntyler wants to merge 5 commits into
mainfrom
bre-1830/skill-workflow-naming-conventions

Conversation

@fntyler

@fntyler fntyler commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🎟️ Tracking

BRE-1830

📔 Objective

Add a workflow-naming-conventions reference skill covering the GitHub Actions naming standards that bwwl does not enforce, so audits stop relying on undocumented convention.

The skill defers to bitwarden-workflow-linter-rules for every linted rule to avoid double-reporting, and treats job ID and file renames as report-only because both are addressable identifiers.

  • Document job IDs (kebab-case), step names (Sentence case with a leading imperative verb), and workflow file names (kebab-case.yml, _ prefix when exclusively reusable)
  • Add reference-sweep procedures for job ID and filename renames, plus an advisory canonical step-name glossary
  • Bump plugin to 0.2.0 with changelog entry and README listing
  • Add greppable to the cspell dictionary

Add a `workflow-naming-conventions` reference skill covering the
GitHub Actions naming standards that `bwwl` does not enforce, so
audits stop relying on undocumented convention.

The skill defers to `bitwarden-workflow-linter-rules` for every
linted rule to avoid double-reporting, and treats job ID and file
renames as report-only because both are addressable identifiers.

* Document job IDs (kebab-case), step names (Sentence case with a
  leading imperative verb), and workflow file names
  (kebab-case.yml, `_` prefix when exclusively reusable)
* Add reference-sweep procedures for job ID and filename renames,
  plus an advisory canonical step-name glossary
* Bump plugin to 0.2.0 with changelog entry and README listing
* Add `greppable` to the cspell dictionary
@fntyler
fntyler requested a review from a team as a code owner August 10, 2026 21:15
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Claude Configuration Validation — PR #192

Result: PASS. No critical or major issues. One plugin changed (bitwarden-devops-engineer, 0.1.5 → 0.2.0) adding one new reference skill. Six minor items below, none blocking merge.

Check Status
Plugin structure & manifest (plugin-validator) ✅ Pass
Skill review (skill-reviewer) ✅ Pass
Config & security (reviewing-claude-config) ✅ Pass
Version bump consistency (4 locations) ✅ Pass
Prettier formatting ✅ Pass (run local)
cspell ✅ Pass (run local)

1. Plugin Validation

Validated plugins/bitwarden-devops-engineer (skills-only plugin — no commands, agents, hooks, or MCP servers).

  • Manifestplugins/bitwarden-devops-engineer/.claude-plugin/plugin.json is valid JSON; name is kebab-case and matches the directory; version 0.2.0 is valid semver; author, homepage, repository, keywords well-formed. Correctly omits a skills key, so auto-discovery from skills/ applies (matches all sibling plugins).
  • Structure — All six skills sit at the auto-discovery path skills/<name>/SKILL.md. No stray files, no empty directories, README present.
  • Version consistency0.2.0 agrees across all four required locations: plugin.json:3, .claude-plugin/marketplace.json:63, root README.md:17 (catalog table), and CHANGELOG.md:8. No agents/ directory exists, so no AGENT.md sync is required. 0.1.5 → 0.2.0 MINOR is the correct semver classification for adding a new skill.
  • Marketplace entrysource: "./plugins/bitwarden-devops-engineer" resolves; key set matches all 15 sibling entries. A repo-wide sweep of all 16 plugins found zero marketplace/README version mismatches.
  • Changelog0.2.0 entry present under ### Added, Keep a Changelog format, describes what changed and why.
  • Credentials — Pattern scan across the plugin for API keys, tokens, passwords, private-key headers, and ghp_/gho_/github_pat_/AKIA prefixes returned nothing. No MCP configuration, so no transport-security surface.

2. Skill Review

Reviewed plugins/bitwarden-devops-engineer/skills/workflow-naming-conventions/SKILL.md (new).

  • Frontmatter — Valid YAML, keys exactly name + description. name: workflow-naming-conventions matches the directory. Description is ~710 chars, under the 1024-char limit.
  • Triggers — Specific and well-chosen; three quoted user phrases are embedded in prose ("what casing should job IDs use", "should this reusable workflow be build.yml or _build.yml", "review these workflows for naming consistency").
  • Length — 111 lines, ~1,290 body words. Inside the 1,000–3,000 target and well under the 500-line limit.
  • Progressive disclosure — Flat single file is appropriate at this size and matches all six skills in the plugin. No references/, examples/, or scripts/ needed yet.
  • Referenced files — The sole external reference, ${CLAUDE_PLUGIN_ROOT}/skills/bitwarden-workflow-linter-rules/SKILL.md (line 16), resolves to an existing file, and the ${CLAUDE_PLUGIN_ROOT} form matches the pattern already used in action-audit/SKILL.md.
  • Style — Consistently imperative/infinitive. Accuracy of every claim about linted rules (name_capitalized, name_exists, underscore_outputs, job_environment_prefix) was verified against the linter-rules skill and holds, with the one scope nit noted below.

3. Configuration & Security Review

  • No settings.json / settings.local.json changed or committed — no permission-scoping or auto-approval surface in this PR.
  • No secrets or hardcoded credentials. The only secret/token regex hits are documentation prose (an "AKV" step-name glossary row and a check-permissions job-ID example).
  • The new skill declares no allowed-tools, granting no tool permissions. This is correct for a reference skill and matches the peer reference skill bitwarden-workflow-linter-rules.
  • Mutating operations described in the skill (git mv for renames, gh search code for the org sweep) appear in prose only and are not auto-approved — consistent with the 0.1.4 security change that removed auto-approved mutating git permissions from this plugin.
  • No malformed definitions: all six SKILL.md files parse cleanly with required fields present.
  • Lint run locally (prettier@3 --check and cspell against .cspell.json): both clean. Prettier reports all changed files match its style; cspell checked 8 plugin markdown files with 0 issues, so the newly introduced terms MSSQL and AKV resolve without a .cspell.json addition. (pnpm is unavailable in this environment, so these were run via npx rather than pnpm run lint.)

Findings

Critical (0)

None.

Major (0)

None.

Minor — warnings (should fix)

  1. plugins/bitwarden-devops-engineer/skills/workflow-naming-conventions/SKILL.md:3-11 — Description has no <example> blocks.
    All five sibling skills in this plugin have exactly two each, including the other pure-reference skill bitwarden-workflow-linter-rules. This is the only skill in the plugin without them. Triggering is likely adequate via the quoted prose phrases, so this is a consistency gap rather than a functional one — but both review agents flagged it as the top item.
    Fix: Add two <example>User: … Action: …</example> pairs matching the sibling pattern. To stay well under the 1024-char limit, trim the trailing "Read alongside bitwarden-workflow-linter-rules, which is the source of truth for linted rules; this skill covers only the gaps" clause (lines 10–11), which already duplicates the Ownership section at lines 14–16.

  2. plugins/bitwarden-devops-engineer/skills/workflow-naming-conventions/SKILL.md:28name_capitalized scope is stated imprecisely.
    The line says "Get Package Version passes the linter and still deviates from the Sentence case standard below." Get Package Version is a step name (it reappears as a step-name example on line 55), but per this plugin's own source of truth (skills/bitwarden-workflow-linter-rules/SKILL.md:26), name_capitalized triggers only on "a workflow-level or job-level name: value" — it never evaluates step names. Saying a step name "passes" implies a linter scope that does not exist, which risks an agent suppressing a step-name finding as already-linted. The conclusion ("The two are complementary") is correct; only the framing is loose.
    Fix: State that name_capitalized does not evaluate step names at all, and that where it does apply it checks only the first character.

  3. plugins/bitwarden-devops-engineer/skills/workflow-audit/SKILL.md (~line 56) — No skill points at the new skill.
    Verified: no SKILL.md in the plugin references workflow-naming-conventions (only the plugin README and CHANGELOG do). The dependency is one-directional — the new skill defers to bitwarden-workflow-linter-rules, and nothing defers back. Since its own description advertises "review these workflows for naming consistency" as a trigger, workflow-audit is the natural entry point, and without a pointer an audit run silently produces zero naming findings.
    Fix: Mention workflow-naming-conventions in the workflow-audit findings step as the owner of non-linted naming findings; optionally add a reciprocal note in bitwarden-workflow-linter-rules.

Minor — nits (optional)

  1. .../workflow-naming-conventions/SKILL.md:48 and :58 — Person inconsistency.
    Line 48 uses the only second-person you in the file ("if you encounter one, fix that first"); line 58 refers to the reader in third person ("This is the only standard here an agent may apply directly"). Both differ from the file's otherwise consistent imperative voice. Suggested: "if one is encountered, fix that first and rename second" and "Apply this standard directly during an edit; it is the only one here that is safe to."

  2. .../workflow-naming-conventions/SKILL.md:8-10 — Garden-path trigger sentence.
    "Use when auditing or authoring workflows and questions like '…', '…', or '…' come up" strands the verb "come up" roughly 40 words after its subject. Suggested rewrite: "Use when auditing or authoring workflows, or when asked what casing job IDs should use, whether a reusable workflow should be build.yml or _build.yml, or to review workflows for naming consistency."

  3. plugins/bitwarden-devops-engineer/CHANGELOG.md:8, 26-40 — Changelog housekeeping (pre-existing / informational).
    Version 0.1.2 is absent from the history — entries jump from ## [0.1.3] (line 26) to ## [0.1.1] (line 40). This predates the PR and is not introduced here. Separately, the 0.2.0 entry is dated 2026-08-10 while today is 2026-08-11; harmless if intentional, worth bumping if the date is meant to track the merge.


Notable Strengths

  • The ownership table (lines 18–26) assigns each naming category to either this skill or the linter by specific rule ID, which is the right defense against double-reporting across two reference skills — reinforced by the explicit "Do not double-report" rule at line 110. Line 26 goes further and names the categories with no standard (inputs, bash variables, artifact names), instructing the agent to flag the gap rather than invent a convention.
  • Blast-radius discipline is well calibrated across three tiers: step names are display-only and safe to edit in place (line 58), job IDs require a three-item reference sweep (lines 42–48), filenames are destructive and belong to the repo owner (line 79). The line 48 caveat — that a job-ID rename is safe unless the job has no name:, in which case the ID becomes the check-run name — is a subtle detail most treatments miss.
  • Line 86 is honest about search coverage, stating plainly that the two searches establish "a floor, not a ceiling" and tying the residual unknown directly to why the rename decision is not the agent's to make.
  • The dual-purpose trigger table (lines 72–77) resolves the _ prefix question deterministically, and line 70 preempts the obvious false positive ("Presence of workflow_call alone never justifies a prefix finding").
  • Advisory content is correctly fenced — line 92 states outright that a step name outside the glossary is never a finding, preventing the table from being read as a whitelist.
  • Version and changelog hygiene is complete across all four required locations, and the plugin README documents the new skill in both the Overview prose and the skills table.

Verdict

Approve. All three validation passes succeeded, and formatting and spell checks were confirmed clean by running the CI tooling locally. The six findings are polish and consistency items; the highest-value follow-ups are adding <example> blocks to the new skill's description (item 1) and tightening the name_capitalized scope statement on line 28 (item 2).

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds a workflow-naming-conventions reference skill to bitwarden-devops-engineer covering job IDs, step name casing, and workflow file naming, plus the 0.2.0 version bump, changelog entry, README listing, and a cspell dictionary addition. The new SKILL.md is 122 lines with valid frontmatter matching the sibling reference skill's format, and every cross-reference to name_capitalized, name_exists, underscore_outputs, and job_environment_prefix matches the definitions in bitwarden-workflow-linter-rules. Version bumps are present in all required files (marketplace.json, plugin.json, root README, changelog); this plugin has no agents, so no AGENT.md updates are needed. The rename-safety guidance (check-run names deriving from name: rather than job ID, step names not being expression-addressable, run history detaching on file rename) is technically accurate.

Code Review Details
  • ❓ : New reference skill has no inbound references from the audit/fix skills, unlike bitwarden-workflow-linter-rules
    • plugins/bitwarden-devops-engineer/README.md:18

@@ -0,0 +1,122 @@
---
name: workflow-naming-conventions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to include something like user-invocable: false for skill definitions of this style/type? Frontmatter reference

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.

Generally speaking, no, but I'm curious how you envision it to be used if it's not. Will it be referenced in another skill or agent somewhere? Also curious why you wouldn't want a user to be able to invoke it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unaware of user-invocable or disable-model-invocation controls for skills prior to working on this one. In this case, this skill is more of a reference (similar to bitwarden-workflow-linter-rules) which I think are better served if invoked by an agent or another skill.

Comment thread plugins/bitwarden-devops-engineer/README.md

@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.

Consider having Skill Creator generate evals to provide evidence that all the prose within the new Skill is actually load-bearing, and not overly verbose as Claude likes to do. Without evals, objective review of the Skill content is impossible unless reviewers run and evaluate it manually.

@@ -0,0 +1,122 @@
---
name: workflow-naming-conventions

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.

Generally speaking, no, but I'm curious how you envision it to be used if it's not. Will it be referenced in another skill or agent somewhere? Also curious why you wouldn't want a user to be able to invoke it.

Comment thread plugins/bitwarden-devops-engineer/skills/workflow-naming-conventions/SKILL.md Outdated
Bring the `workflow-naming-conventions` skill description in line
with the skill spec, which forbids XML tags in `description`.

Trigger phrasings are folded into prose so the skill still matches
the same potential user questions.
Disambiguate this skill's rule IDs from real `bwwl` rule IDs. The
three standards are code-fenced headings styled exactly like linter
rules, and only kebab-vs-snake casing separates them, so a reader of
a generated report could mistake a convention finding for linter
output.
Address the ownership line's source-of-truth pointer with a
resolvable path instead of a bare skill name, matching the form
`action-audit` uses to reach the same skill.
Align the two sentence case headings in `workflow-naming-conventions`
with the Title Case used across the plugin's other skill headings.
Cosmetic only.

* `## Advisory: canonical step names` to `## Advisory: Canonical Step Names`
* `## Applying these standards` to `## Applying These Standards`
@fntyler
fntyler requested a review from SaintPatrck August 11, 2026 17:28
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.

2 participants