Skip to content

Breakdown plan skill#143

Merged
trmartin4 merged 44 commits into
mainfrom
tech-breakdown-plan-skill
Jun 19, 2026
Merged

Breakdown plan skill#143
trmartin4 merged 44 commits into
mainfrom
tech-breakdown-plan-skill

Conversation

@trmartin4

@trmartin4 trmartin4 commented Jun 10, 2026

Copy link
Copy Markdown
Member

📔 Objective

Adds the next in a sequence of changes to decompose and enhance the existing writing-tech-breakdowns skill, for use with our new tech-breakdowns repo.

The plan for skill decomposition is roughly defined below, and this is the 3rd skill in that plan:

The plan for skill decomposition is roughly defined below, and these are the first 2 skills in that plan:

For current ideation on all the remaining skills, you can reference #134, which will not be merged in its current state

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Plugin Validation Report — PR #143

Validated: bitwarden-delivery-tools (v1.5.0) and bitwarden-tech-lead (v2.3.1)
Checks run: plugin structure validation, skill quality review, security scan.

✅ Overall: PASS

Both plugins are structurally sound, all skill frontmatter is well-formed, all referenced files exist, versions are consistent across manifests, and no credentials or insecure patterns were found. No critical or major errors. A few minor/informational notes are listed below.


1. Plugin Structure Validation

Check delivery-tools (1.5.0) tech-lead (2.3.1)
plugin.json valid JSON, name matches dir, valid semver
Version consistent with marketplace.json ✅ (line 81) ✅ (line 69)
README.md present
CHANGELOG.md present, Keep a Changelog format, entry for current version [1.5.0] - 2026-06-17 [2.3.1] - 2026-06-10
Skill auto-discovery (name matches directory) ✅ all skills ✅ all skills
Agent frontmatter (name/description+<example>/model/color/prompt) n/a (no agents) AGENT.md fully valid
Hooks / MCP config none (not required) none (not required)
Hardcoded credentials ✅ none ✅ none

No errors.

Minor (should fix)

  • plugins/bitwarden-tech-lead/CHANGELOG.md:5 — Keep a Changelog reference URL is …/en/1.1.0/, whereas bitwarden-delivery-tools/CHANGELOG.md:5 and the convention use …/en/1.0.0/. Cosmetic only; the changelog format itself is correct. Align for consistency.

2. Skill Quality Review

Reviewed: developing-breakdown-plan, developing-breakdown-spec, starting-breakdown (delivery-tools), and architecting-solutions (tech-lead).

Skill Body size Frontmatter Refs valid Verdict
developing-breakdown-plan ~2.0k words references/process-flow.dot Pass
developing-breakdown-spec ~1.0k words references/process-flow.dot Pass
starting-breakdown ~0.7k words n/a (no refs) Pass
architecting-solutions ~1.4k words n/a (no refs) Pass

Strengths: All four have specific, concrete trigger phrases; name matches directory; imperative/infinitive writing style throughout; good progressive disclosure (the two complex skills push their full decision graph to references/process-flow.dot). None are bloated.

Minor (should fix)

  • Cross-plugin Skill() referencesarchitecting-solutions/SKILL.md:75 invokes Skill(navigating-the-initiative-funnel), which lives in the sibling bitwarden-delivery-tools plugin (verified to exist), not in bitwarden-tech-lead. This resolves only when both plugins are installed. This is consistent with the established cross-plugin pattern already documented in the tech-lead CHANGELOG (e.g. 2.1.0, 2.2.0), so it is intentional — but worth confirming the inter-plugin dependency is documented in the README, or softening to prose if standalone install must work. Skill(contributing-to-technical-strategy) (line 55) is in-plugin and fine.

Informational (no action required)

  • The skills declare extra frontmatter fields (argument-hint, arguments, allowed-tools). These are non-standard for portable skills but match this repo's established convention across existing skills — not a defect.
  • AskUserQuestion / TaskCreate are used in skill bodies but not listed in allowed-tools. Repo convention is mixed here; these are built-in, ungated tools, so behavior is unaffected. Optionally declare them for auditability/consistency.

⚠️ False positive corrected

An automated reviewer pass flagged "stray </output> tags" at the end of the three delivery-tools skills. This was verified and is incorrectgrep over plugins/bitwarden-delivery-tools/skills returns no output tags. The flag was an artifact of the file-reader's own output wrapper, not file content. No action needed.


3. Security Validation

Check Result
settings.local.json committed ✅ not present in changed files
Hardcoded secrets / API keys / tokens / passwords ✅ none — manifests are metadata only; skills reference MCP tools by name
Dangerous command auto-approvals ✅ none
allowed-tools Bash scoping ✅ scoped (see below)
Shell command construction (injection) ✅ explicitly guarded
Untrusted external content handling ✅ exemplary

Bash scoping is tight and read-only where it matters:

  • developing-breakdown-plan: Bash(gh pr list:*), Bash(git log:*), Bash(grep:*) — all read-only, sub-command scoped.
  • starting-breakdown: Bash(git clone:*), Bash(git pull:*), Bash(git status:*), Bash(cp:*), Bash(mkdir:*)cp/mkdir are the broadest grants, but the skill body (line 51) mitigates with strict input validation: slug must match ^[a-z][a-z0-9-]*$, Jira key ^[A-Z][A-Z0-9]+-[0-9]+$, with explicit rejection of non-validated values before interpolation.

Injection defense is a standout positive:

  • developing-breakdown-plan:33 — mandates binding untrusted-derived values as literal positional args (grep -F -- "$NAME"), never splicing into shell-evaluated strings.
  • starting-breakdown:51 — regex-validates slug/Jira key before any mkdir/cp.
  • Every skill (developing-breakdown-plan:32, developing-breakdown-spec:30, starting-breakdown:28, architecting-solutions:15) explicitly treats Jira/Confluence/PR/branch content as untrusted data, not instructions — strong prompt-injection awareness appropriate for a security-sensitive repo.

No security issues found.

Informational (no action required)

  • Bash(cp:*) / Bash(mkdir:*) in starting-breakdown are wildcard-scoped and could in principle target arbitrary paths; this is acceptably mitigated by the in-body validation described above. Noted for awareness only.

Summary of Action Items

All items are minor / optional — none block the PR.

  1. (Minor) Align bitwarden-tech-lead/CHANGELOG.md:5 Keep a Changelog URL to …/en/1.0.0/ for cross-plugin consistency.
  2. (Minor) Confirm the cross-plugin dependency (architecting-solutionsnavigating-the-initiative-funnel in bitwarden-delivery-tools) is documented in the tech-lead README, or soften the Skill() reference if standalone install is a requirement.
  3. (Optional) Decide a consistent policy on declaring built-in AskUserQuestion/TaskCreate in skill allowed-tools.

Base automatically changed from tech-breakdown-spec-skill to main June 15, 2026 15:47
@trmartin4 trmartin4 marked this pull request as ready for review June 15, 2026 17:05
@trmartin4 trmartin4 requested a review from a team as a code owner June 15, 2026 17:05
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds the developing-breakdown-plan skill to bitwarden-delivery-tools (3rd in the tech-breakdown decomposition sequence) and refines the architecting-solutions skill in bitwarden-tech-lead. Both plugins are version-bumped correctly (1.4.0→1.4.1, 2.3.0→2.3.1) with marketplace/README/plugin.json kept in sync, and the new skill's referenced files (references/process-flow.dot, references/worked-example.md) exist. The new skill's untrusted-data handling and hard-gate orientation are well-constructed. One process-compliance gap noted below.

Code Review Details
  • ♻️ : Two substantive architecting-solutions edits (new "Avoid deprecated methods" bullet, removed "Accepting an initiative epic..." bullet) are missing from the 2.3.1 CHANGELOG entry
    • plugins/bitwarden-tech-lead/skills/architecting-solutions/SKILL.md:32

Comment thread plugins/bitwarden-tech-lead/skills/architecting-solutions/SKILL.md Outdated

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

⚠️ Adding new skills should be a minor version bump; not patch bump. We have been fairly consistent with that across plugins. Will you please bump bitwarden-delivery-tools to 1.5.0?

❓/💡 Maybe not a now thing, but going back-n-forth between developing-breakdown-spec and developing-breakdown-plan is a little confusing. Trying to draw the connections and review this one while going back to what's in it's predecessor/sibling was difficult 😵‍💫

Regardless, handful of things to consider refactoring/clarifying. Couple general questions/ideas. Lemme know if you decide to defer any of the changes until after we get tech leads using the new skills.

Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
Comment thread plugins/bitwarden-tech-lead/CHANGELOG.md
@trmartin4

trmartin4 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

❓/💡 Maybe not a now thing, but going back-n-forth between developing-breakdown-spec and developing-breakdown-plan is a little confusing. Trying to draw the connections and review this one while going back to what's in it's predecessor/sibling was difficult 😵‍💫

@theMickster Do you think it makes sense to combine spec + plan skills into one then? They are closely related and build upon one another; "doing a breakdown" implies both.

I have addressed all other feedback, I believe. Thank you for the thorough review!

@trmartin4 trmartin4 requested a review from theMickster June 18, 2026 01:21
Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
<HARD-GATE>
Prompt the user to switch to their workspace root: the folder containing their local clone of `tech-breakdowns/` alongside the other Bitwarden repos (`server/`, `clients/`, `sdk-internal/`, `ios/`, `android/`, etc.). The skill relies on traversing those siblings to scan in-flight work and resolve cross-team impact.

Orientation within a breakdown is required. Ask the user which breakdown to work against. They can give a path, a Jira key, or a team/slug — use `Glob` under `tech-breakdowns/` to resolve to a real `breakdown.md`. If the user already named it earlier in the conversation, confirm the resolved path with `AskUserQuestion` before proceeding.

@SaintPatrck SaintPatrck Jun 18, 2026

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.

🎨 Would be helpful to coerce this proactively when invoked by adding argument-hints and/or arguments in frontmatter. Then the document can be referenced with $ARGUMENTS[n] or $name within the Skill body.

More on argument-hints and arguments can be found here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you for the docs! I think I've used that correctly with the changes in 78090f1 (this PR). I wasn't aware this was something you could do!

Comment thread plugins/bitwarden-delivery-tools/skills/developing-breakdown-plan/SKILL.md Outdated
@trmartin4 trmartin4 requested a review from SaintPatrck June 18, 2026 21:10

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

I'm am good with this round of changes. Anything else at the moment feels too ⛏️-esk

@trmartin4 trmartin4 merged commit cc67176 into main Jun 19, 2026
13 checks passed
@trmartin4 trmartin4 deleted the tech-breakdown-plan-skill branch June 19, 2026 10:49
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.

3 participants