fix(skills): CodeQL code-quality — missing comma, empty except, unused import by risleylima · Pull Request #107 · bmad-code-org/bmad-builder

risleylima · 2026-08-09T22:15:29Z

Summary

Fixes #106. Clears the five GitHub Code Quality (CodeQL) findings that consumer repos inherit after installing BMad Builder skills under .agents/skills.

What was wrong

1. Implicit string concatenation in a list (`py/implicit-string-concatenation-in-list`) — warning

File: skills/bmad-agent-builder/assets/init-sanctum-template.py

Inside lines.extend([...]), two adjacent string literals sat next to each other without a comma:

"Prefer crafting your own tools over depending on external ones. A script you wrote "
"and saved is more reliable than an external API. Use the file system creatively.",

In a Python list/tuple display, that is implicit concatenation (one list element), which CodeQL flags because it usually means a missing comma between list items.

Fix: add the comma after the first string so they are two list elements (joined later by "\n".join(lines)):

"Prefer crafting your own tools over depending on external ones. A script you wrote ",
"and saved is more reliable than an external API. Use the file system creatively.",

2. Empty `except` without comment (`py/empty-except`) — note ×3

CodeQL requires an explanatory comment when an except body is only pass.

File	Intent
`bmad-module-builder/scripts/scaffold-standalone-module.py`	`module.yaml` metadata is optional; keep marketplace defaults if unreadable
`bmad-*-builder/scripts/render_report.py` (agent + workflow copies)	Best-effort unlink of temp file before re-raising the original error

Fix: keep behavior; add one-line comments documenting the intentional swallow.

3. Unused import (`py/unused-import`) — note

File: skills/bmad-workflow-builder/scripts/tests/test_canon_sync.py

import sys was never used (__main__ only prints and asserts).

Fix: remove the import.

Test plan

Diff review: only the five skill files above
Spot-check generate_capabilities_md Tools section still renders as two consecutive lines after "\n".join
python3 skills/bmad-workflow-builder/scripts/tests/test_canon_sync.py (self-check) still passes when canon copies are present
After merge + consumer reinstall/update of BMB skills, Code Quality findings Broken ref: brief-template.md referenced with wrong path depth #12–Broken ref: agent validate step referenced as step-01-validate.md instead of v-01-load-review.md #16 (or equivalents) clear on next analysis

Notes

These issues were byte-identical on main / v2.1.0; a BMAD quick-update alone would not have cleared them without this source fix.

Made with Cursor

Summary by CodeRabbit

Documentation
- Added clarifying comments describing error handling and fallback behavior.
- Documented that temporary-file cleanup is best effort while original errors are preserved.
Chores
- Removed an unused import from the test suite.
- No user-facing behavior changes.

Add the missing list comma to avoid implicit string concatenation, document intentional empty except clauses, and drop unused sys import. Fixes bmad-code-org#106. Co-authored-by: Cursor <cursoragent@cursor.com>

greptile-apps

Your trial has ended. Reactivate Greptile to resume code reviews.

coderabbitai · 2026-08-09T22:15:51Z

Walkthrough

The changes address CodeQL quality findings across builder skills. They add comments for intentional exception handling, preserve generated output, remove an unused import, and leave runtime behavior unchanged.

Changes

Code quality cleanup

Layer / File(s)	Summary
Code quality annotations and import cleanup `skills/bmad-agent-builder/...`, `skills/bmad-module-builder/...`, `skills/bmad-workflow-builder/...`	Comments document best-effort cleanup and preserved marketplace defaults. The template output remains unchanged. The unused `sys` import is removed.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: bmadcode

Poem

A rabbit checks each builder line,
And leaves the error paths clear and fine.
Whitespace hops, imports flee,
Comments guard the code gently.
“Clean and quiet,” says the bunny.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The comments and import removal address four findings, but the implicit string concatenation finding remains unresolved in the template file.	Fix the adjacent string literals in init-sanctum-template.py with the intended explicit separator, then verify that CodeQL reports no remaining findings.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the CodeQL quality fixes and names the affected finding types.
Out of Scope Changes check	✅ Passed	All changes are limited to the files and CodeQL findings identified in linked issue `#106`.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@skills/bmad-agent-builder/assets/init-sanctum-template.py`:
- Line 170: Update the Tools guidance entries in the generated lines list so the
fragments ending with “wrote” and beginning with “and saved” remain one list
element without relying on implicit string concatenation. Preserve the resulting
single-line output when "\n".join(lines) generates CAPABILITIES.md.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20b0551e-188f-4e5a-8f6e-3c9fdb8f1c5b

📥 Commits

Reviewing files that changed from the base of the PR and between e6935f2 and 2edeaa8.

📒 Files selected for processing (5)

skills/bmad-agent-builder/assets/init-sanctum-template.py
skills/bmad-agent-builder/scripts/render_report.py
skills/bmad-module-builder/scripts/scaffold-standalone-module.py
skills/bmad-workflow-builder/scripts/render_report.py
skills/bmad-workflow-builder/scripts/tests/test_canon_sync.py

💤 Files with no reviewable changes (1)

skills/bmad-workflow-builder/scripts/tests/test_canon_sync.py

coderabbitai · 2026-08-09T22:17:21Z

        "## Tools",
        "",
-        "Prefer crafting your own tools over depending on external ones. A script you wrote "
+        "Prefer crafting your own tools over depending on external ones. A script you wrote ",


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the Tools guidance as one generated line.

The comma on Line 170 makes Line 171 a separate lines element. "\n".join(lines) inserts a newline between wrote and and saved, which changes the generated CAPABILITIES.md content.

Keep both fragments in one list element while avoiding implicit string concatenation.

Proposed fix

- "Prefer crafting your own tools over depending on external ones. A script you wrote ", - "and saved is more reliable than an external API. Use the file system creatively.", + ( + "Prefer crafting your own tools over depending on external ones. " + + "A script you wrote and saved is more reliable than an external API. " + + "Use the file system creatively." + ),

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

"Prefer crafting your own tools over depending on external ones. A script you wrote ",

(

"Prefer crafting your own tools over depending on external ones. "

"A script you wrote and saved is more reliable than an external API. "

"Use the file system creatively."

),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/bmad-agent-builder/assets/init-sanctum-template.py` at line 170, Update the Tools guidance entries in the generated lines list so the fragments ending with “wrote” and beginning with “and saved” remain one list element without relying on implicit string concatenation. Preserve the resulting single-line output when "\n".join(lines) generates CAPABILITIES.md.

fix(skills): clear CodeQL code-quality findings in Python builders

2edeaa8

Add the missing list comma to avoid implicit string concatenation, document intentional empty except clauses, and drop unused sys import. Fixes bmad-code-org#106. Co-authored-by: Cursor <cursoragent@cursor.com>

greptile-apps Bot reviewed Aug 9, 2026

View reviewed changes

coderabbitai Bot reviewed Aug 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

Uh oh!

fix(skills): CodeQL code-quality — missing comma, empty except, unused import - #107

fix(skills): CodeQL code-quality — missing comma, empty except, unused import#107
risleylima wants to merge 1 commit into
bmad-code-org:mainfrom
risleylima:fix/codeql-python-quality-skills

risleylima commented Aug 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

greptile-apps Bot left a comment

Uh oh!

coderabbitai Bot commented Aug 9, 2026 •

edited

Loading

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Aug 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-        "Prefer crafting your own tools over depending on external ones. A script you wrote ",
+        (
+            "Prefer crafting your own tools over depending on external ones. "
+             "A script you wrote and saved is more reliable than an external API. "
+             "Use the file system creatively."
+        ),

Uh oh!

Uh oh!

Conversation

risleylima commented Aug 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What was wrong

1. Implicit string concatenation in a list (py/implicit-string-concatenation-in-list) — warning

2. Empty except without comment (py/empty-except) — note ×3

3. Unused import (py/unused-import) — note

Test plan

Notes

Summary by CodeRabbit

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

risleylima commented Aug 9, 2026 •

edited by coderabbitai Bot

Loading

1. Implicit string concatenation in a list (`py/implicit-string-concatenation-in-list`) — warning

2. Empty `except` without comment (`py/empty-except`) — note ×3

3. Unused import (`py/unused-import`) — note

coderabbitai Bot commented Aug 9, 2026 •

edited

Loading