fix: correct 3 broken file refs and enforce validator as CI quality gate#1717
Conversation
- create-architecture/workflow.md: fix installed_path dir name from 'architecture' to 'create-architecture' - create-story/checklist.md: fix 2 refs from validate-workflow.xml to workflow.xml (file does not exist with validate- prefix) - package.json: add --strict to validate:refs so broken references fail CI instead of logging warnings and exiting 0
🤖 Augment PR SummarySummary: Fixes three broken workflow/documentation file references and updates the 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThree configuration and documentation files are updated to tighten file reference validation and standardize workflow framework references. The validate:refs npm script is enhanced with a --strict flag, and workflow files are updated to use the new workflow.xml framework instead of validate-workflow.xml. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/bmm/workflows/4-implementation/create-story/checklist.md (2)
189-189:⚠️ Potential issue | 🟡 MinorDuplicate "Step 5" labels create navigational ambiguity for AI agents.
The document contains two distinct sections each labeled "Step 5" — one in the SYSTEMATIC RE-ANALYSIS APPROACH and one in the INTERACTIVE IMPROVEMENT PROCESS. Since AI agents executing workflows navigate by step label, a step-by-label lookup will be ambiguous or resolve to whichever Step 5 appears first in context. The second block's steps should be renumbered relative to its own section, or the section headers should be disambiguated (e.g., "Phase 2, Step 5").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/checklist.md` at line 189, The document has two sections both labeled "Step 5" causing ambiguous step-by-label navigation; update the second occurrence inside the "INTERACTIVE IMPROVEMENT PROCESS" (or its sub-steps) so labels are unique—either renumber its steps (e.g., to "Step 1/Step 2..." within that section or continue the global sequence) or disambiguate the section header (e.g., "Phase 2 — Step 5") to ensure any step-by-label lookup (references to "Step 5" in the SYSTEMATIC RE-ANALYSIS APPROACH and INTERACTIVE IMPROVEMENT PROCESS) maps unambiguously to a single target.
54-54:⚠️ Potential issue | 🟠 MajorMissed fix: line 54 still names the non-existent
validate-workflow.xml.Lines 36 and 66 were updated to
workflow.xml, but line 54 in the Required Inputs block was not. An AI agent that reads this section to determine which framework file to load now sees two contradictory names inside the same document. Sincevalidate-workflow.xmldoes not exist, any agent that selects its framework from the Required Inputs list will silently fail to load the framework.This bare filename was invisible to the file-reference validator because the validator resolves full-path patterns, not standalone filenames — which is also a systematic gap worth documenting separately (see the
package.jsoncomment).🐛 Proposed fix
- - **Validation framework**: `validate-workflow.xml` (handles checklist execution) + - **Validation framework**: `workflow.xml` (handles checklist execution)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/checklist.md` at line 54, Update the "Required Inputs" block in the checklist to remove the obsolete filename `validate-workflow.xml` and replace it with `workflow.xml` so it matches the other references (lines previously changed at 36 and 66); specifically edit the "Validation framework" entry in the Required Inputs section (the text showing `validate-workflow.xml`) to read `workflow.xml` to avoid agents trying to load a non-existent framework file.package.json (2)
51-65:⚠️ Potential issue | 🟠 Major
lint-stagedhas novalidate:refshook — broken refs can be committed without local feedback.The
*.mdentry only runsmarkdownlint-cli2. A developer editing a workflow.mdfile and introducing a broken file reference will not be warned at commit time; the error surfaces only in CI after a push.Consider adding a lint-staged entry for
.mdfiles that runsvalidate:refs(accepting that it will re-scan all refs, not just staged files, which is a known limitation of whole-repo validators):🛡️ Proposed fix
"*.md": [ - "markdownlint-cli2" + "markdownlint-cli2", + "npm run validate:refs" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 51 - 65, The lint-staged config for "*.md" only runs "markdownlint-cli2" and misses the repository-level refs validator, so add the "npm run validate:refs" step to the "*.md" array in package.json's "lint-staged" block (accepting that validate:refs is a whole-repo check); update the "*.md" entry to run both "markdownlint-cli2" and "npm run validate:refs" so broken file references are caught locally before commit.
43-46:⚠️ Potential issue | 🟠 MajorTwo independent ref-checking tools with no documented relationship — false safety is possible.
test:refs(CSV extraction testing,test/test-file-refs-csv.js) is in thetestscript and tests only theextractCsvRefs()function.validate:refs(comprehensive ref validation,tools/validate-file-refs.js --strict) is defined but not in thetestscript. The latter validates all reference patterns across YAML, markdown, XML, CSV, relative paths, exec attributes, invoke-task directives, step metadata, Load directives, and absolute path leaks.A broken reference that
test:refsdoes not cover (for example, an invalidexec="..."attribute in YAML or a broken relative path in markdown) will silently passnpm testbecausevalidate:refs --strictis never executed. This divergence in scope undermines the enforcement guarantee. Untilvalidate:refsis merged into the test script or explicitly documented as a separate validation tier, the test suite creates a false sense of confidence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 43 - 46, The test script currently runs "test:refs" (which only exercises extractCsvRefs()) but omits "validate:refs" (tools/validate-file-refs.js --strict), allowing many ref errors to slip by; update package.json so the top-level "test" script includes "validate:refs" (e.g., add an npm run validate:refs step after "test:refs") so the comprehensive validation is executed as part of npm test, or alternatively add a clear documented note in README explaining that "validate:refs" is a separate, required validation tier if you intentionally keep it out of the test flow.
🧹 Nitpick comments (1)
package.json (1)
48-48: The--strictflag is correctly implemented — but validation is enforced only in CI, not in the local development loop.The validator properly parses and uses the
--strictflag (it transforms warnings into errors). However,validate:refsis invoked only in the CI pipeline (line 116 of.github/workflows/*.yml), not innpm testorlint-staged. Developers running the standard local test loop (npm test) receive no feedback on broken file references until the PR reaches CI.While this is not a systematic enforcement failure, it creates friction: developers must wait for CI feedback to catch reference issues locally detectable by
npm run validate:refs.Optional: wire validate:refs into npm test for tighter local feedback
- "test": "npm run test:schemas && npm run test:refs && npm run test:install && npm run validate:schemas && npm run lint && npm run lint:md && npm run format:check", + "test": "npm run test:schemas && npm run test:refs && npm run test:install && npm run validate:schemas && npm run validate:refs && npm run lint && npm run lint:md && npm run format:check",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 48, Add the validate:refs script into the local dev test loop so file-ref validation runs outside CI: update package.json to invoke the existing "validate:refs" (the npm script name) as part of the "test" command (or add it as a "pretest" script) and/or include it in the lint-staged or pre-commit hooks so developers get immediate feedback; reference the "validate:refs" npm script and modify the "test" (or "pretest") script entry in package.json to run npm run validate:refs before the existing test steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 48: The CI script "validate:refs" currently passes the --strict flag
which gives a false sense of full coverage because tools/validate-file-refs.js
intentionally skips validating the "{installed_path}" interpolation; either
remove the --strict flag from the "validate:refs" npm script in package.json to
avoid gating on partial validation, or extend tools/validate-file-refs.js (the
validation logic) to treat "{installed_path}" patterns as validatable (add
parsing/lookup or explicit checks) and update docs/README to call out the new
coverage so maintainers know the limitation is addressed.
---
Outside diff comments:
In `@package.json`:
- Around line 51-65: The lint-staged config for "*.md" only runs
"markdownlint-cli2" and misses the repository-level refs validator, so add the
"npm run validate:refs" step to the "*.md" array in package.json's "lint-staged"
block (accepting that validate:refs is a whole-repo check); update the "*.md"
entry to run both "markdownlint-cli2" and "npm run validate:refs" so broken file
references are caught locally before commit.
- Around line 43-46: The test script currently runs "test:refs" (which only
exercises extractCsvRefs()) but omits "validate:refs"
(tools/validate-file-refs.js --strict), allowing many ref errors to slip by;
update package.json so the top-level "test" script includes "validate:refs"
(e.g., add an npm run validate:refs step after "test:refs") so the comprehensive
validation is executed as part of npm test, or alternatively add a clear
documented note in README explaining that "validate:refs" is a separate,
required validation tier if you intentionally keep it out of the test flow.
In `@src/bmm/workflows/4-implementation/create-story/checklist.md`:
- Line 189: The document has two sections both labeled "Step 5" causing
ambiguous step-by-label navigation; update the second occurrence inside the
"INTERACTIVE IMPROVEMENT PROCESS" (or its sub-steps) so labels are unique—either
renumber its steps (e.g., to "Step 1/Step 2..." within that section or continue
the global sequence) or disambiguate the section header (e.g., "Phase 2 — Step
5") to ensure any step-by-label lookup (references to "Step 5" in the SYSTEMATIC
RE-ANALYSIS APPROACH and INTERACTIVE IMPROVEMENT PROCESS) maps unambiguously to
a single target.
- Line 54: Update the "Required Inputs" block in the checklist to remove the
obsolete filename `validate-workflow.xml` and replace it with `workflow.xml` so
it matches the other references (lines previously changed at 36 and 66);
specifically edit the "Validation framework" entry in the Required Inputs
section (the text showing `validate-workflow.xml`) to read `workflow.xml` to
avoid agents trying to load a non-existent framework file.
---
Nitpick comments:
In `@package.json`:
- Line 48: Add the validate:refs script into the local dev test loop so file-ref
validation runs outside CI: update package.json to invoke the existing
"validate:refs" (the npm script name) as part of the "test" command (or add it
as a "pretest" script) and/or include it in the lint-staged or pre-commit hooks
so developers get immediate feedback; reference the "validate:refs" npm script
and modify the "test" (or "pretest") script entry in package.json to run npm run
validate:refs before the existing test steps.
2 participants
What
--strictmode on the file reference validator so broken references now fail CI instead of logging warnings and exiting 0Why
The validator (
tools/validate-file-refs.js) has always supported--strictmode but was invoked without it in CI, making it advisory-only. Broken references could be merged undetected indefinitely.Three references were broken and confirmed by running the validator in strict mode:
src/bmm/workflows/3-solutioning/create-architecture/workflow.mdline 39 —installed_pathreferencedbmm/workflows/3-solutioning/architecturebut the directory is namedcreate-architecturesrc/bmm/workflows/4-implementation/create-story/checklist.mdlines 36 and 66 — referencedcore/tasks/validate-workflow.xmlwhich does not exist; the actual file iscore/tasks/workflow.xmlHow
--stricttovalidate:refsinpackage.json— no changes needed in the CI workflow YAML itself, which already callsnpm run validate:refsRelated
Fixes the enforcement gap described in #1626.