Follow-ups from the review of #114 (merged). That PR fixed all three symptoms reported in #113; these are adjacent defects found during review that were deliberately left out of scope rather than holding a correct fix.
Listed in priority order.
1. bmad-workflow-builder carries the same bug, unfixed
skills/bmad-workflow-builder/scripts/scan-path-standards.py is a near-verbatim sibling of the agent-builder copy and still has both original defects:
:119 — md_file.name != 'SKILL.md', so the root .memlog.md is flagged as a structure violation:207 — unfiltered rglob, so generated .analysis/ output is scanned
It has the same contract that makes those false positives: bmad-workflow-builder/SKILL.md:28 requires a root .memlog.md, and references/scan-orchestration.md:7 writes .analysis/<timestamp>/. The bug reproduces verbatim there.
#113's bug class is only half-closed. Applying #114's three changes to this copy should close it.
2. .memlog.md is exempt from the structure rule but still content-scanned
#114 removes .memlog.md from check_root_md_files but leaves it in md_files, so scan_file still runs all six path regexes over its contents.
An ordinary memlog entry such as:
- (decision) sanctum lives at _bmad/memory/bmad-agent-sentinel/
still produces two high findings (bare-bmad, memory-path) and keeps the scan at status: fail.
Worth being precise about severity: this is probabilistic, not deterministic. It fires only when memlog prose happens to contain trigger strings, unlike the structure finding #114 removed, which fired on 100% of runs because the file always exists. A spot check of 12 real .memlog.md files found 2 tripping bare_bmad — and those came from other skills, not agent-builder, whose vocabulary is consistently _bmad/memory/{skillName}/.
This is CodeRabbit's one unresolved inline comment on #114, and it is correct.
3. The .analysis filter is unanchored (latent)
scan_skill filters with '.analysis' not in path.parts, which tests the absolute path rather than the path relative to the skill root. If any ancestor directory is named .analysis, every file in the skill is filtered out and the scanner reports status: pass, 0 findings, exit 0 — on a skill it never read.
Demonstrated on a deliberately dirty agent placed at <tmp>/.analysis/myagent:
BASELINE: status fail, 7 findings, 17 files scanned
MERGED: Warning: No .md or .json files found
status pass, 0 findings, 0 files scanned, exit 0
Not reachable in practice — the scanner is always invoked as scan-path-standards.py {target-agent-path} against an agent skill directory, and .analysis/ is only ever created inside a target skill. Producing this nesting requires hand-placing an agent inside a prior analysis output. So it's a latent trap for the next refactorer, not a live hazard.
A false PASS on a lint gate is the dangerous failure direction, and the fix is one line:
if '.analysis' not in path.relative_to(skill_path).parts
That is exactly what #114's own other half already does at prepass.py:128 (path.relative_to(root).parts) — the two halves are currently inconsistent with each other.
4. prepass.py counts .memlog.md in its file inventory (documentation drift)
references/quality-analysis.md:49 states the pre-pass "never reads the builder's .memlog.md." It does — iter_files yields it into the files list the lenses reason about. Classification itself is unaffected (has_sanctum reads only SKILL.md text plus globs), so nothing is misclassified. Either the code or the doc should change.
Testing note
There is no test suite for bmad-agent-builder (scripts/tests does not exist), and nothing references scan-path-standards.py or prepass.py outside their own docstrings. #114 shipped without tests because there is nowhere to put them. Any of the above would benefit from being the first.
More broadly: package.json has no test script, and .github/workflows/quality.yaml runs only prettier, markdownlint and docs build — no Python test in this repo is executed by CI. Several suites also use hyphenated test-*.py filenames that default pytest discovery will not collect. See #88 for a test that has been red on main since May as a direct result.
🤖 Filed by BMad Bot following the review of #114.
Follow-ups from the review of #114 (merged). That PR fixed all three symptoms reported in #113; these are adjacent defects found during review that were deliberately left out of scope rather than holding a correct fix.
Listed in priority order.
1.
bmad-workflow-buildercarries the same bug, unfixedskills/bmad-workflow-builder/scripts/scan-path-standards.pyis a near-verbatim sibling of the agent-builder copy and still has both original defects::119—md_file.name != 'SKILL.md', so the root.memlog.mdis flagged as a structure violation:207— unfilteredrglob, so generated.analysis/output is scannedIt has the same contract that makes those false positives:
bmad-workflow-builder/SKILL.md:28requires a root.memlog.md, andreferences/scan-orchestration.md:7writes.analysis/<timestamp>/. The bug reproduces verbatim there.#113's bug class is only half-closed. Applying #114's three changes to this copy should close it.
2.
.memlog.mdis exempt from the structure rule but still content-scanned#114 removes
.memlog.mdfromcheck_root_md_filesbut leaves it inmd_files, soscan_filestill runs all six path regexes over its contents.An ordinary memlog entry such as:
still produces two high findings (
bare-bmad,memory-path) and keeps the scan atstatus: fail.Worth being precise about severity: this is probabilistic, not deterministic. It fires only when memlog prose happens to contain trigger strings, unlike the structure finding #114 removed, which fired on 100% of runs because the file always exists. A spot check of 12 real
.memlog.mdfiles found 2 trippingbare_bmad— and those came from other skills, not agent-builder, whose vocabulary is consistently_bmad/memory/{skillName}/.This is CodeRabbit's one unresolved inline comment on #114, and it is correct.
3. The
.analysisfilter is unanchored (latent)scan_skillfilters with'.analysis' not in path.parts, which tests the absolute path rather than the path relative to the skill root. If any ancestor directory is named.analysis, every file in the skill is filtered out and the scanner reportsstatus: pass, 0 findings, exit 0 — on a skill it never read.Demonstrated on a deliberately dirty agent placed at
<tmp>/.analysis/myagent:Not reachable in practice — the scanner is always invoked as
scan-path-standards.py {target-agent-path}against an agent skill directory, and.analysis/is only ever created inside a target skill. Producing this nesting requires hand-placing an agent inside a prior analysis output. So it's a latent trap for the next refactorer, not a live hazard.A false PASS on a lint gate is the dangerous failure direction, and the fix is one line:
That is exactly what #114's own other half already does at
prepass.py:128(path.relative_to(root).parts) — the two halves are currently inconsistent with each other.4.
prepass.pycounts.memlog.mdin its file inventory (documentation drift)references/quality-analysis.md:49states the pre-pass "never reads the builder's.memlog.md." It does —iter_filesyields it into thefileslist the lenses reason about. Classification itself is unaffected (has_sanctumreads onlySKILL.mdtext plus globs), so nothing is misclassified. Either the code or the doc should change.Testing note
There is no test suite for
bmad-agent-builder(scripts/testsdoes not exist), and nothing referencesscan-path-standards.pyorprepass.pyoutside their own docstrings. #114 shipped without tests because there is nowhere to put them. Any of the above would benefit from being the first.More broadly:
package.jsonhas notestscript, and.github/workflows/quality.yamlruns only prettier, markdownlint and docs build — no Python test in this repo is executed by CI. Several suites also use hyphenatedtest-*.pyfilenames that default pytest discovery will not collect. See #88 for a test that has been red onmainsince May as a direct result.🤖 Filed by BMad Bot following the review of #114.