feat(quick-dev): render templates via stdlib Python at skill entry#2281
Conversation
752c086 to
94cf189
Compare
94cf189 to
550807f
Compare
3d07364 to
c1c6db9
Compare
0c08cdd to
93ff8d4
Compare
Move compile-time variable substitution out of the LLM and into a deterministic Python step. SKILL.md becomes a two-line stdout-dispatch shim that runs render.py and follows the instruction it prints. The renderer reads BMad configuration from the central four-layer TOML surface introduced in #2285 (_bmad/config.toml plus config.user.toml and the two _bmad/custom/ overrides), with a fallback to the legacy per-module _bmad/bmm/config.yaml for pre-#2285 installs. Compile-time refs ({{.var}}) get substituted at render time. LLM-runtime refs ({var}) pass through untouched. Renderer (render.py) - Python 3 stdlib only (tomllib, already bundled since 3.11). UTF-8 I/O. Every invocation rebuilds from scratch — no hash, no cache. - find_project_root walks up from cwd; HALT to stdout if no _bmad/ is found anywhere on the path. - load_central_config deep-merges the four TOML layers in priority order (base-team → base-user → custom-team → custom-user) so user overrides in _bmad/custom/config.user.toml win over installer- regenerated base values. flatten_central_config lifts scalar keys from [core] and [modules.bmm] into the renderer's flat namespace; module keys beat core on collision (matches the installer's own core-key-stripping behavior). - When _bmad/config.toml is absent, falls through to the legacy flat-YAML parser for _bmad/bmm/config.yaml — the renderer keeps working across the #2285 transition. - {{.var}} substitution; unresolved refs emit empty string (Go missingkey=zero semantics). - Smart defaults for planning_artifacts / implementation_artifacts / communication_language applied after config load. Derives sprint_status / deferred_work_file from implementation_artifacts. {{.main_config}} points at whichever surface was actually read. - Renders every .md in the skill dir except SKILL.md to {project-root}/_bmad/render/bmad-quick-dev/. - On success, stderr summary plus a single stdout line: "read and follow {workflow_md}". On failure, stdout HALT directive — per the Anthropic skills spec, script stdout is the defined agent- communication channel. Skill entry (SKILL.md) - Two-line shim: run python render.py, follow stdout. No template tokens in SKILL.md itself. Template conversions - workflow.md, step-01..05, step-oneshot, sync-sprint-status: convert every compile-time {var} reference to {{.var}}. Runtime refs preserved. - spec-template.md untouched (single-curly comment hint stays as documentation). Skill-prose cleanups bundled in - Remove dead step-file frontmatter: empty-string variable declarations (spec_file, story_key, diff_output, review_mode) in quick-dev step-01 and code-review step-01; empty --- --- blocks in step-03 and step-05; the specLoopIteration counter init moved from step-04 frontmatter into the step body where first-entry vs loopback semantics are explicit. - Unify the language rule across all six quick-dev step files plus workflow.md. Tooling - tools/validate-skills.js: add TPL-01 rule. Files whose name contains "template" must not contain compile-time {{.var}} substitutions. Template files seed durable, version-controlled artifacts that execute on other machines; baking a value at render time would freeze a machine-local path into every downstream artifact. - tools/validate-file-refs.js: add render/ to INSTALL_ONLY_PATHS so the validator recognizes the runtime-generated buffer. - tools/skill-validator.md: document TPL-01; deterministic rule count bumped from 14 to 15. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single happy path: central _bmad/config.toml with four-layer merge, Python 3.11+ required (no ImportError guard), HALT if config missing. Deletes load_flat_yaml, the YAML fallback branch, the setdefault block for planning_artifacts/implementation_artifacts/communication_language, and the tomllib ImportError fallback. Part of plan-quick-dev-python-config-hardening.md (F0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On Windows, os.path.join returns backslash-separated paths that can misrender as escape sequences when later concatenated into POSIX shell strings or regexes. Normalize the project root to forward slashes after find_project_root, and use posixpath.join for every path that gets baked into rendered .md files or joined into config values. os.makedirs and os.listdir accept forward-slash paths on Windows, so their call sites stay as-is. Part of plan-quick-dev-python-config-hardening.md (F3). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Python text-mode open() with the platform default performs universal- newline translation: on Windows, LF source files get written as CRLF, producing spurious diffs when rendered output is compared against source. Pass newline="" on both the source read and the rendered write so line endings pass through verbatim. Part of plan-quick-dev-python-config-hardening.md (F4). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
render.py rebuilds from scratch per the docstring, but makedirs(exist_ok=True) only overwrites files that still exist in the source — stale outputs from renamed/deleted source files linger in _bmad/render/bmad-quick-dev/ forever. Remove every .md in the render dir before the render loop; keep the dir itself and any non-.md files. Part of plan-quick-dev-python-config-hardening.md (F5). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous INSTALL_ONLY_PATHS entry 'render/' was a blanket prefix
that let every {project-root}/_bmad/render/... reference in any skill
slip past validation. Narrow to 'render/bmad-quick-dev/' so only this
skill's render buffer is whitelisted. Future skills adopting the
stdout-dispatch renderer pattern add their own entries explicitly.
Part of plan-quick-dev-python-config-hardening.md (F6).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New test/test-quick-dev-renderer.js spins up a temp project with base _bmad/config.toml and a _bmad/custom/config.user.toml override, runs render.py, and asserts the override wins in rendered workflow.md and that sprint_status is rooted at an absolute path in the temp project. Registered as test:renderer in package.json and chained into the npm test script. Part of plan-quick-dev-python-config-hardening.md (F7). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Load the four config layers through a load_toml helper that marks the base _bmad/config.toml as required. A missing, unparseable, or unreadable base now prints a HALT directive to stdout and exits, instead of being silently skipped and then crashing downstream with a KeyError when a derived value (e.g. implementation_artifacts) is absent. Optional layers still warn on stderr and fall back to empty. Merge semantics are unchanged (dict-aware deep merge, override wins for lists and scalars).
The bare `python render.py` shim assumes the agent's working directory is
the skill directory, but agents run from the project root, so the script
is not found. Reference it as `{skill-root}/render.py` — BMAD's standard
token for a skill's installed directory, already used by every other
skill's resolve_customization.py invocation — and add the one-line
`{skill-root}` explainer so the model resolves it from an instruction
rather than guessing. Interpreter stays `python`; the python vs python3
choice is a separate cross-platform concern.
render.py now merges the three customize layers (customize.toml ->
custom/bmad-quick-dev.toml -> .user.toml) with the same structural rules as
resolve_customization.py and inlines the resolved [workflow] values, so no
{workflow.*} placeholder survives. workflow.md drops its Step 1 runtime
resolver + manual-merge fallback; step-05 and step-oneshot drop their runtime
workflow.on_complete calls. The shared resolve_customization.py and every
other skill are untouched. Smoke test extended with a [workflow] override
fixture covering inlining, array append, and no-leak assertions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The shim called bare `python`, which can resolve to Python 2 or be absent; render.py needs 3.11+ for tomllib. Spell out python3 and the version requirement. Also make the exit code authoritative: on a non-zero exit (including an uncaught crash that writes only to stderr), do not proceed -- report what was printed and stop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "rendered N files" progress line was pure diagnostic noise. The shim already tells the LLM to ignore stderr and follow the stdout instruction, so on success render.py now prints only the "read and follow ..." line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…orkflow The gate ported from #2398 defended against runtime customization indirection: agents guessed resolver outputs instead of executing them, silently skipping append steps. render.py inlines the prepend/append entries into the rendered workflow.md, so there is nothing left to short-circuit, and each inlined list already carries its own execute- in-order imperative. In the default install both lists render as _None._ and the gate is pure noise.
Reconcile #2550 with render-time [workflow] resolution. Main made review layers configurable as [[workflow.review_layers]] arrays of tables and had the LLM resolve them during activation; this branch resolves the [workflow] block in render.py instead, so activation-time resolution no longer exists and the layer refs must be materialized at render time. Rather than inlining the layer tables as data plus interpretation rules, render.py now knows this skill's customization schema outright and renders review_layers/oneshot_review_layers as direct invocation blocks: disabled layers (empty instruction) drop out, each active layer becomes a #### section holding its instruction verbatim, zero active layers renders the HALT instruction, and runtime placeholders like {diff_output} pass through. The only judgment left to the LLM is the optional `when` condition, which renders as a run-time guard line. The step-04/step-oneshot review intros collapse to a single execute-in- parallel imperative. Smoke test covers default rendering, replace-by-id, disable-by-empty-instruction, when-guards, and the all-disabled HALT.
93ff8d4 to
118a8c8
Compare
The SKILL.md shim launched render.py with bare `python3`, which the rest of BMAD is migrating away from: the customize-bmad docs and the installer's uv-check standardize on `uv run` (uv provisions a suitable 3.11+ interpreter on demand). Bare `python3` is also fragile on Windows, where python.org installs expose `python`/`py` rather than `python3`. Make `uv run` the primary invocation and demote `python3` to the documented fallback, spelling out `python`/`py -3` for Windows and the 3.11+ tomllib requirement. render.py itself is unchanged; the renderer test drives it directly and is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
🤖 Augment PR SummarySummary: This PR makes Changes:
Why: Reduce LLM drift by making compile-time variable resolution deterministic and enable clearer project/team overrides via config/customization layers. 🤖 Was this summary useful? React with 👍 or 👎 |
| Module keys take precedence on collision (installer strips core keys from | ||
| module buckets, so collisions shouldn't happen in practice).""" | ||
| flat = {} | ||
| for section in (merged.get("core"), merged.get("modules", {}).get("bmm")): |
There was a problem hiding this comment.
src/bmm-skills/4-implementation/bmad-quick-dev/render.py:191 — merged.get("modules", {}).get("bmm") will raise if modules exists but isn’t a dict (e.g., malformed/partial TOML), which would crash the renderer instead of producing a deterministic stdout HALT.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| vars_["project_root"] = root | ||
| vars_["main_config"] = posixpath.join(bmad_dir, "config.toml") | ||
| vars_["sprint_status"] = posixpath.join( |
There was a problem hiding this comment.
src/bmm-skills/4-implementation/bmad-quick-dev/render.py:304 — Deriving sprint_status/deferred_work_file assumes implementation_artifacts is present in vars_; if it’s missing, this throws KeyError and the script won’t emit the expected stdout HALT instruction.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| ### Step 3: Load Config | ||
|
|
||
| Load config from `{{.main_config}}` and resolve: |
There was a problem hiding this comment.
src/bmm-skills/4-implementation/bmad-quick-dev/workflow.md:54 — This says to load config from {{.main_config}}, but render.py sets main_config to _bmad/config.toml (one layer), so runtime reads may miss config.user.toml / custom/* overrides and potentially contradict already-baked values like {{.communication_language}}.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ); | ||
| }); | ||
|
|
||
| test('disabling every layer renders the HALT instruction', () => { |
There was a problem hiding this comment.
test/test-quick-dev-renderer.js:242 — The “disabling every layer renders the HALT instruction” test hard-codes a specific set of default review-layer IDs; adding a new default layer (or renaming an id) would make this test fail even if the renderer is still correct.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
📝 WalkthroughWalkthroughAdds a Python render.py script that compiles TOML config/customization into rendered Markdown for the bmad-quick-dev skill, migrates SKILL.md and step files to {{.var}}/{workflow.*} placeholder syntax, adds a smoke test and npm wiring, and adds a TPL-01 validation rule plus a validator path exclusion. ChangesQuick Dev Renderer Pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Skill Validation Tooling Update
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/bmm-skills/4-implementation/bmad-quick-dev/render.py (1)
290-336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new quick-dev render pipeline
Add a note in the relevant docs (especiallydocs/explanation/quick-dev.mdanddocs/how-to/customize-bmad.md) thatbmad-quick-devnow renders templates viarender.py, resolves workflow placeholders at compile time, and writes generated output under_bmad/render/bmad-quick-dev/.🤖 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 `@src/bmm-skills/4-implementation/bmad-quick-dev/render.py` around lines 290 - 336, Update the quick-dev documentation to describe the new render pipeline implemented in main() within render.py: it now loads and flattens central config, resolves workflow placeholders at compile time via resolve_workflow/render_workflow, and writes generated markdown into _bmad/render/bmad-quick-dev/. Add this note in the relevant docs such as docs/explanation/quick-dev.md and docs/how-to/customize-bmad.md, and make sure the wording clearly reflects that bmad-quick-dev is rendered rather than manually edited.Source: Path instructions
🤖 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 `@src/bmm-skills/4-implementation/bmad-quick-dev/render.py`:
- Around line 290-311: main() currently assumes
vars_["implementation_artifacts"] always exists, which can raise a raw KeyError
when config is missing that key. Update the path construction for sprint_status
and deferred_work_file to validate the presence of implementation_artifacts
first, and use the same friendly HALT-style error handling already used by
find_project_root and load_toml so the script fails cleanly instead of
tracebacking.
---
Nitpick comments:
In `@src/bmm-skills/4-implementation/bmad-quick-dev/render.py`:
- Around line 290-336: Update the quick-dev documentation to describe the new
render pipeline implemented in main() within render.py: it now loads and
flattens central config, resolves workflow placeholders at compile time via
resolve_workflow/render_workflow, and writes generated markdown into
_bmad/render/bmad-quick-dev/. Add this note in the relevant docs such as
docs/explanation/quick-dev.md and docs/how-to/customize-bmad.md, and make sure
the wording clearly reflects that bmad-quick-dev is rendered rather than
manually edited.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 392ffb2d-6b6e-472d-8503-72921051aae7
📒 Files selected for processing (15)
package.jsonsrc/bmm-skills/4-implementation/bmad-quick-dev/SKILL.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/render.pysrc/bmm-skills/4-implementation/bmad-quick-dev/step-01-clarify-and-route.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/step-02-plan.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/step-03-implement.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/step-04-review.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/step-05-present.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/sync-sprint-status.mdsrc/bmm-skills/4-implementation/bmad-quick-dev/workflow.mdtest/test-quick-dev-renderer.jstools/skill-validator.mdtools/validate-file-refs.jstools/validate-skills.js
| def main(): | ||
| script_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| skill_name = os.path.basename(script_dir) | ||
| root = find_project_root() | ||
| root = root.replace(os.sep, "/") | ||
| bmad_dir = posixpath.join(root, "_bmad") | ||
|
|
||
| vars_ = flatten_central_config(load_central_config(root)) | ||
|
|
||
| for key in list(vars_.keys()): | ||
| vars_[key] = vars_[key].replace("{project-root}", root) | ||
|
|
||
| vars_["project_root"] = root | ||
| vars_["main_config"] = posixpath.join(bmad_dir, "config.toml") | ||
| vars_["sprint_status"] = posixpath.join( | ||
| vars_["implementation_artifacts"], "sprint-status.yaml" | ||
| ) | ||
| vars_["deferred_work_file"] = posixpath.join( | ||
| vars_["implementation_artifacts"], "deferred-work.md" | ||
| ) | ||
|
|
||
| workflow = resolve_workflow(root, script_dir.replace(os.sep, "/"), skill_name) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unhandled KeyError if implementation_artifacts is absent from config.
vars_["implementation_artifacts"] (Lines 304-306, 307-309) is read unconditionally to build sprint_status/deferred_work_file, but flatten_central_config only populates keys that happen to exist in [core]/[modules.bmm] (Lines 186-199) — there's no guarantee implementation_artifacts is present. Unlike the rest of this file, which consistently HALTs with a clear stdout message on missing/invalid input (find_project_root, load_toml), a missing implementation_artifacts key here raises a raw KeyError traceback instead of a friendly HALT — the exact failure mode this script otherwise avoids by design.
🛡️ Proposed fix
+ if "implementation_artifacts" not in vars_:
+ print(
+ "HALT and report to the user: config is missing required key "
+ "`implementation_artifacts` (expected under [core] or [modules.bmm])"
+ )
+ sys.exit(1)
+
vars_["project_root"] = root📝 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.
| def main(): | |
| script_dir = os.path.dirname(os.path.abspath(__file__)) | |
| skill_name = os.path.basename(script_dir) | |
| root = find_project_root() | |
| root = root.replace(os.sep, "/") | |
| bmad_dir = posixpath.join(root, "_bmad") | |
| vars_ = flatten_central_config(load_central_config(root)) | |
| for key in list(vars_.keys()): | |
| vars_[key] = vars_[key].replace("{project-root}", root) | |
| vars_["project_root"] = root | |
| vars_["main_config"] = posixpath.join(bmad_dir, "config.toml") | |
| vars_["sprint_status"] = posixpath.join( | |
| vars_["implementation_artifacts"], "sprint-status.yaml" | |
| ) | |
| vars_["deferred_work_file"] = posixpath.join( | |
| vars_["implementation_artifacts"], "deferred-work.md" | |
| ) | |
| workflow = resolve_workflow(root, script_dir.replace(os.sep, "/"), skill_name) | |
| def main(): | |
| script_dir = os.path.dirname(os.path.abspath(__file__)) | |
| skill_name = os.path.basename(script_dir) | |
| root = find_project_root() | |
| root = root.replace(os.sep, "/") | |
| bmad_dir = posixpath.join(root, "_bmad") | |
| vars_ = flatten_central_config(load_central_config(root)) | |
| for key in list(vars_.keys()): | |
| vars_[key] = vars_[key].replace("{project-root}", root) | |
| if "implementation_artifacts" not in vars_: | |
| print( | |
| "HALT and report to the user: config is missing required key " | |
| "`implementation_artifacts` (expected under [core] or [modules.bmm])" | |
| ) | |
| sys.exit(1) | |
| vars_["project_root"] = root | |
| vars_["main_config"] = posixpath.join(bmad_dir, "config.toml") | |
| vars_["sprint_status"] = posixpath.join( | |
| vars_["implementation_artifacts"], "sprint-status.yaml" | |
| ) | |
| vars_["deferred_work_file"] = posixpath.join( | |
| vars_["implementation_artifacts"], "deferred-work.md" | |
| ) | |
| workflow = resolve_workflow(root, script_dir.replace(os.sep, "/"), skill_name) |
🤖 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 `@src/bmm-skills/4-implementation/bmad-quick-dev/render.py` around lines 290 -
311, main() currently assumes vars_["implementation_artifacts"] always exists,
which can raise a raw KeyError when config is missing that key. Update the path
construction for sprint_status and deferred_work_file to validate the presence
of implementation_artifacts first, and use the same friendly HALT-style error
handling already used by find_project_root and load_toml so the script fails
cleanly instead of tracebacking.
render.py derived sprint_status/deferred_work_file from an unconditional
vars_["implementation_artifacts"] subscript, so a config lacking that key
raised a raw KeyError instead of the stdout HALT the rest of the script
uses on bad input. flatten_central_config likewise called .get("bmm") on
merged["modules"] without checking it was a table, so a non-table
[modules] crashed with an AttributeError.
Guard both: HALT with a clear stdout directive when implementation_artifacts
is missing or blank, and coerce a non-dict modules to {} before indexing.
Add renderer regression tests asserting each path exits without a Python
traceback.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… re-read
The activation "Load Config" step told the LLM to open {{.main_config}}
and re-resolve project_name, communication_language, sprint_status, etc.
at run time -- but render.py already bakes those from the full four-layer
config merge. main_config pointed at only the base _bmad/config.toml, so
on installs with override layers (config.user.toml / custom/*) the runtime
re-read saw stale values that could contradict the baked {{.var}} in the
same rendered file. It also handed resolution back to the LLM: the drift
this skill's renderer exists to remove.
Delete the ceremony and wire each value where it is actually used:
- Every value the step resolved is already inlined at its point of use
(planning/implementation_artifacts, sprint_status, communication_language)
or loaded via persistent_facts (project-context.md), so the central
block was pure redundancy.
- Fold document_output_language into the per-step language rule, adopting
the house-canonical form ("Speak in X. Write any file output in Y.")
already used by bmad-checkpoint-preview.
- Move the {date} = current-datetime definition to step-02, where the
spec template's {date} field is filled.
- Drop the user greeting (user_name) and user_skill_level tailoring:
quick-dev is not a conversational skill and neither was load-bearing.
- Remove main_config from render.py; it had no remaining consumer.
Renderer tests repointed at the files that now carry these values, plus
coverage for document_output_language baking and main_config removal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rlies
Curlies mean "expand this to the value"; a bare backticked name means
"this is the variable/field I'm talking about". Several step files wrapped
a variable in curlies where they were only naming, assigning, passing, or
testing it -- so the notation implied an expansion that never happens:
- step-01: identify `epic_num`/`story_num`, set/leave `story_key` unset
- step-02: test `preserved_intent`; and resolve the template's `date` field
(was `{date}`, which read as "expand date here" rather than naming it)
- step-03/step-05/step-oneshot: pass `target_status` to sync-sprint-status,
set `title`
- sync-sprint-status: the `target_status` parameter, `story_key` precondition,
and both `target_status` conditionals
Value tokens that are genuinely materialized in place -- `{spec_file}` paths,
`development_status[{story_key}]`, "set ... to `{target_status}`" -- stay
curly. Also reword step-02's frozen-block instruction from the ambiguous
"substitute it for the `<frozen-after-approval>` block" to "replace the
`<frozen-after-approval>` block in the spec you just filled out with
`preserved_intent`" so it's clear the replacement happens in the artifact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The SKILL.md shim tried `uv run render.py` and, if uv was missing, retried with a bare `python3`/`py -3` interpreter. Nothing else in the codebase does that interpreter fallback: the uv-based skills (bmad-prd, bmad-ux, bmad-architecture, bmad-product-brief) fall back to reading customize.toml and using defaults -- graceful feature degradation, never a different runner -- and the legacy skills just call python3 outright. uv is the established house runner (memlog.py, resolve_customization.py, lint_spine.py all invoke it). That graceful-degrade path does not exist here: render.py is the entry dispatch that produces the workflow.md the LLM then follows, so there is nothing to fall back to. The only honest outcomes are "uv runs it" or "HALT". Make uv the floor and drop the fallback. Pin the interpreter the house way -- a PEP 723 `requires-python = ">=3.11"` block, matching memlog.py/lint_spine.py -- so `uv run` provisions a 3.11+ interpreter and the tomllib requirement is guaranteed rather than hoped for. This replaces the prose "needs 3.11+" hedge the shim used to carry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dee43eb to
7684c17
Compare
What
Move compile-time variable substitution and
[workflow]customization resolution out of the LLM and into a deterministic Python step, and delete the leftover runtime config-resolution ceremony it made redundant.SKILL.mdbecomes a short stdout-dispatch shim that runsrender.pyand follows the instruction it prints. The renderer walks up to find_bmad/, loads config from a four-layer TOML merge, bakes{project-root}to an absolute path, resolves the skill's[workflow]customization block (materializing review layers into direct invocation blocks), and writes rendered.mdfiles to_bmad/render/bmad-quick-dev/.Two placeholder classes are resolved at render time — compile-time refs (
{{.var}}) and workflow-customization refs ({workflow.<key>}). LLM-runtime refs (single-curly{var}such as{project-root},{spec_file},{skill-root}) pass through untouched. After this PR there is no runtime config re-read: every config value is baked once, at its point of use.Why
Before, the LLM resolved
{project-root},{communication_language},{implementation_artifacts},{sprint_status}, etc. at runtime by reading config, and separately ranresolve_customization.pyto interpret the[workflow]block. That pushed substitution and customization-merge onto the LLM (fragile: it can misresolve or drift), left no clean place for project-level overrides, and conflated compile-time refs with LLM-runtime refs.Moving compile-time substitution and customization resolution into a deterministic Python step eliminates LLM drift, materializes review-layer semantics into concrete invocation blocks instead of leaving the LLM to interpret them, and opens the door for other skills to adopt the same pattern later.
How
Renderer (
render.py)tomllib). UTF-8 I/O. Every invocation rebuilds from scratch — no hash, no cache.find_project_rootwalks up from cwd; HALT to stdout if no_bmad/is found._bmad/config.toml(required) +config.user.toml+custom/config.toml+custom/config.user.toml, highest priority last (post-refactor(skills): remove bmad-skill-manifest yaml; introduce central config.toml #2285 install layout). Scalar keys are flattened from[core]and[modules.bmm]into one namespace.{{.var}}substitution; unresolved refs emit an empty string (Gomissingkey=zerosemantics).{project-root}is baked to an absolute path across all values;sprint_status/deferred_work_fileare derived fromimplementation_artifacts.[workflow]customization: three-layer merge —{skill}/customize.toml+_bmad/custom/bmad-quick-dev.toml+.user.toml, using the same structural deep-merge semantics asresolve_customization.py(tables deep-merge; arrays-of-tables keyed bycode/idreplace-then-append; scalars override). Resolved values fill{workflow.<key>}placeholders, so this skill needs no runtimeresolve_customization.pycall.[[workflow.review_layers]]/oneshot_review_layersrender into direct#### <name>invocation blocks. A layer with an empty/missinginstructionis treated as disabled (how an override turns off a default layer) and drops out; awhencondition stays with the LLM as a runtime guard line; no active layers renders as an explicit HALT block.config.tomlmissing or unparseable, a missing/blankimplementation_artifacts, and a non-table[modules]all print a clear stdout HALT directive and exit non-zero, matching the contract the rest of the script already followed..mdin the skill dir exceptSKILL.md→_bmad/render/bmad-quick-dev/(stale renders cleared first). On success, prints a single stdout line:read and follow {workflow_md}.Skill entry (
SKILL.md)uv run {skill-root}/render.py, then follow stdout.uvis the required runner — no interpreter fallback (consistent with the otheruv-based skills; the graceful-degrade-to-defaults path other skills use does not apply here, sincerender.pyis the entry dispatch).render.pycarries a PEP 723requires-python = ">=3.11"block (matchingmemlog.py/lint_spine.py), souv runprovisions a 3.11+ interpreter and thetomllibrequirement is guaranteed. No template tokens inSKILL.mditself — per the Anthropic skills spec, script stdout is the defined agent-communication channel.Config wired at point-of-use, not re-resolved at runtime
The old activation flow had the LLM "Load config from
{{.main_config}}and resolve" a list of keys at runtime. Butmain_configpointed at only the base_bmad/config.toml, so on installs with override layers the runtime re-read saw stale values that could contradict the baked{{.var}}in the same rendered file — reintroducing exactly the LLM drift this renderer exists to remove. That whole step is now deleted:planning_artifacts/implementation_artifacts,sprint_status,communication_language) or loaded viapersistent_facts(project-context.md), so the central block was pure redundancy.document_output_languagefolds into a single per-step language rule, adopting the house-canonical form already used bybmad-checkpoint-preview:Speak in {{.communication_language}}. Write any file output in {{.document_output_language}}.across all six step files. This also closes the prior gap wheredocument_output_languagewas declared but never enforced on file writes.{date}= current-datetime definition moves tostep-02, where the spec template's{date}field is actually filled.main_configis removed fromrender.py— it had no remaining consumer.user_name) anduser_skill_leveltailoring are dropped: quick-dev is a non-conversational dev skill and neither was load-bearing.Template conversions
workflow.md,step-01..05,step-oneshot,sync-sprint-status: compile-time{var}references converted to{{.var}}, customization routed through{workflow.<key>}. Runtime refs preserved.spec-template.mduntouched (single-curly comment hint stays as documentation; protected by the new TPL-01 validator rule).Skill-prose cleanups bundled in
---/---blocks); move thespecLoopIterationinit fromstep-04frontmatter into the step body where first-entry vs loopback semantics are explicit.workflow.mdto its load-bearing parts and fold in the materialized review-layer invocation blocks.Tooling
tools/validate-skills.js— add TPL-01 rule: files whose name containstemplatemust not contain compile-time{{.var}}substitutions (baking a value would freeze a machine-local path into every downstream artifact). HIGH severity.tools/validate-file-refs.js— addrender/toINSTALL_ONLY_PATHSso the validator recognizes the runtime-generated buffer.tools/skill-validator.md— document TPL-01; deterministic rule count 14 → 15.Testing
npm ci && npm run qualitypasses — full suite (lint, markdownlint, docs build, test:install, test:renderer, validate:refs, validate:skills). Skills validator strict-mode pass, no findings in files touched here.test/test-quick-dev-renderer.js— 19 renderer tests covering TOML override resolution,{{.var}}/{workflow.*}substitution,{project-root}baking,document_output_languagebaking into the per-step language rule, review-layer materialization, the all-layers-disabled HALT path,main_configremoval, and the bad-config HALT paths (missingimplementation_artifacts, non-table[modules]) asserting each exits without a Python traceback.uv run render.pyfrom a harness cwd produces expected output; all{{.var}}/{workflow.*}placeholders resolved; runtime single-curly refs preserved.Follow-ups deferred
Resolved along the way (previously listed here as deferred): the flat-YAML parser is gone (now
tomllibagainst post-#2285_bmad/config.toml); missing/blankimplementation_artifactsand non-table[modules]now HALT cleanly; the runtime config-layer parity gap is closed by removing the runtime re-read entirely.Still open, tracked in harness
deferred-work.md:docs/explanation/quick-dev.md/docs/how-to/customize-bmad.md.render.pyhardening: atomic writes, broader error handling, regex hyphen handling.