Breakdown plan skill#143
Conversation
Plugin Validation Report — PR #143Validated: ✅ Overall: PASSBoth 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
No errors. Minor (should fix)
2. Skill Quality ReviewReviewed:
Strengths: All four have specific, concrete trigger phrases; Minor (should fix)
Informational (no action required)
|
| 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/mkdirare 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 anymkdir/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:*)instarting-breakdownare 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.
- (Minor) Align
bitwarden-tech-lead/CHANGELOG.md:5Keep a Changelog URL to…/en/1.0.0/for cross-plugin consistency. - (Minor) Confirm the cross-plugin dependency (
architecting-solutions→navigating-the-initiative-funnelinbitwarden-delivery-tools) is documented in the tech-lead README, or soften theSkill()reference if standalone install is a requirement. - (Optional) Decide a consistent policy on declaring built-in
AskUserQuestion/TaskCreatein skillallowed-tools.
Co-authored-by: Mick Letofsky <mick.tosk@gmail.com>
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR adds the Code Review Details
|
theMickster
left a comment
There was a problem hiding this comment.
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.
@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! |
| <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. |
There was a problem hiding this comment.
🎨 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.
There was a problem hiding this comment.
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!
theMickster
left a comment
There was a problem hiding this comment.
I'm am good with this round of changes. Anything else at the moment feels too ⛏️-esk
📔 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