Skip to content

Commit 1040c3c

Browse files
tyagiakhileshAkhilesh Tyagibmadcode
authored
fix: correctly resolve output_folder paths outside project root (#2132)
* fix(bmad-init): correctly resolve output_folder paths outside project root When output_folder was set to an absolute path (e.g. /Users/me/outputs), the {project-root}/{value} result template stored it as {project-root}//absolute/path. resolve_project_root_placeholder then did a naive string replace, producing /project//absolute/path — a broken path that workflows could not resolve. For relative paths outside the root (e.g. ../../sibling), the same naive replace left un-normalized paths like /project/../../sibling in the resolved config, which some tools mishandled. Fix resolve_project_root_placeholder to strip the {project-root} token, detect whether the remainder is absolute (returning it directly) or relative (joining with project root and normalizing via os.path.normpath). Fix apply_result_template to skip the template entirely when raw_value is already an absolute path, and to normalize the result for relative-but- outside paths. This covers the bmad-init SKILL write path, which bakes the resolved path directly into config.yaml. Add 7 tests covering all three path cases (absolute, relative-with- traversal, normal in-project) for both functions. * Address review comments --------- Co-authored-by: Akhilesh Tyagi <akhilesh.t@nextiva.com> Co-authored-by: Brian <bmadcode@gmail.com>
1 parent ed9dea9 commit 1040c3c

File tree

3 files changed

+113
-5
lines changed

3 files changed

+113
-5
lines changed

docs/how-to/non-interactive-installation.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,19 @@ Requires [Node.js](https://nodejs.org) v20+ and `npx` (included with npm).
3737
| `--user-name <name>` | Name for agents to use | System username |
3838
| `--communication-language <lang>` | Agent communication language | English |
3939
| `--document-output-language <lang>` | Document output language | English |
40-
| `--output-folder <path>` | Output folder path | _bmad-output |
40+
| `--output-folder <path>` | Output folder path (see resolution rules below) | `_bmad-output` |
41+
42+
#### Output Folder Path Resolution
43+
44+
The value passed to `--output-folder` (or entered interactively) is resolved according to these rules:
45+
46+
| Input type | Example | Resolved as |
47+
|------------|---------|-------------|
48+
| Relative path (default) | `_bmad-output` | `<project-root>/_bmad-output` |
49+
| Relative path with traversal | `../../shared-outputs` | Normalized absolute path — e.g. `/Users/me/shared-outputs` |
50+
| Absolute path | `/Users/me/shared-outputs` | Used as-is — project root is **not** prepended |
51+
52+
The resolved path is what agents and workflows use at runtime when writing output files. Using an absolute path or a traversal-based relative path lets you direct all generated artifacts to a directory outside your project tree — useful for shared or monorepo setups.
4153

4254
### Other Options
4355

@@ -141,6 +153,7 @@ Invalid values will either:
141153

142154
:::tip[Best Practices]
143155
- Use absolute paths for `--directory` to avoid ambiguity
156+
- Use an absolute path for `--output-folder` when you want artifacts written outside the project tree (e.g. a shared monorepo outputs directory)
144157
- Test flags locally before using in CI/CD pipelines
145158
- Combine with `-y` for truly unattended installations
146159
- Use `--debug` if you encounter issues during installation

src/core-skills/bmad-init/scripts/bmad_init.py

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,27 @@ def resolve_project_root_placeholder(value, project_root):
166166
"""Replace {project-root} placeholder with actual path."""
167167
if not value or not isinstance(value, str):
168168
return value
169-
if '{project-root}' in value:
170-
return value.replace('{project-root}', str(project_root))
171-
return value
169+
if '{project-root}' not in value:
170+
return value
171+
172+
# Strip the {project-root} token to inspect what remains, so we can
173+
# correctly handle absolute paths stored as "{project-root}//absolute/path"
174+
# (produced by the "{project-root}/{value}" template applied to an absolute value).
175+
suffix = value.replace('{project-root}', '', 1)
176+
177+
# Strip the one path separator that follows the token (if any)
178+
if suffix.startswith('/') or suffix.startswith('\\'):
179+
remainder = suffix[1:]
180+
else:
181+
remainder = suffix
182+
183+
if os.path.isabs(remainder):
184+
# The original value was an absolute path stored with a {project-root}/ prefix.
185+
# Return the absolute path directly — no joining needed.
186+
return remainder
187+
188+
# Relative path: join with project root and normalize to resolve any .. segments.
189+
return os.path.normpath(os.path.join(str(project_root), remainder))
172190

173191

174192
def parse_var_specs(vars_string):
@@ -222,9 +240,22 @@ def apply_result_template(var_def, raw_value, context):
222240
if not result_template:
223241
return raw_value
224242

243+
# If the user supplied an absolute path and the template would prefix it with
244+
# "{project-root}/", skip the template entirely to avoid producing a broken path
245+
# like "/my/project//absolute/path".
246+
if isinstance(raw_value, str) and os.path.isabs(raw_value):
247+
return raw_value
248+
225249
ctx = dict(context)
226250
ctx['value'] = raw_value
227-
return expand_template(result_template, ctx)
251+
result = expand_template(result_template, ctx)
252+
253+
# Normalize the resulting path to resolve any ".." segments (e.g. when the user
254+
# entered a relative path such as "../../outside-dir").
255+
if isinstance(result, str) and '{' not in result and os.path.isabs(result):
256+
result = os.path.normpath(result)
257+
258+
return result
228259

229260

230261
# =============================================================================

src/core-skills/bmad-init/scripts/tests/test_bmad_init.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,37 @@ def test_none(self):
110110
def test_non_string(self):
111111
self.assertEqual(resolve_project_root_placeholder(42, Path('/test')), 42)
112112

113+
def test_absolute_path_stored_with_prefix(self):
114+
"""Absolute output_folder entered by user is stored as '{project-root}//abs/path'
115+
by the '{project-root}/{value}' template. It must resolve to '/abs/path', not
116+
'/project//abs/path'."""
117+
result = resolve_project_root_placeholder(
118+
'{project-root}//Users/me/outside', Path('/Users/me/myproject')
119+
)
120+
self.assertEqual(result, '/Users/me/outside')
121+
122+
def test_relative_path_with_traversal_is_normalized(self):
123+
"""A relative path like '../../sibling' produces '{project-root}/../../sibling'
124+
after the template. It must resolve to the normalized absolute path, not the
125+
un-normalized string '/project/../../sibling'."""
126+
result = resolve_project_root_placeholder(
127+
'{project-root}/../../sibling', Path('/Users/me/myproject')
128+
)
129+
self.assertEqual(result, '/Users/sibling')
130+
131+
def test_relative_path_one_level_up(self):
132+
result = resolve_project_root_placeholder(
133+
'{project-root}/../outside-outputs', Path('/project/root')
134+
)
135+
self.assertEqual(result, '/project/outside-outputs')
136+
137+
def test_standard_relative_path_unchanged(self):
138+
"""Normal in-project relative paths continue to work correctly."""
139+
result = resolve_project_root_placeholder(
140+
'{project-root}/_bmad-output', Path('/project/root')
141+
)
142+
self.assertEqual(result, '/project/root/_bmad-output')
143+
113144

114145
class TestExpandTemplate(unittest.TestCase):
115146

@@ -147,6 +178,39 @@ def test_value_only_template(self):
147178
result = apply_result_template(var_def, 'English', {})
148179
self.assertEqual(result, 'English')
149180

181+
def test_absolute_value_skips_project_root_template(self):
182+
"""When the user enters an absolute path, the '{project-root}/{value}' template
183+
must not be applied — doing so would produce '/project//absolute/path'."""
184+
var_def = {'result': '{project-root}/{value}'}
185+
result = apply_result_template(
186+
var_def, '/Users/me/shared-outputs', {'project-root': '/Users/me/myproject'}
187+
)
188+
self.assertEqual(result, '/Users/me/shared-outputs')
189+
190+
def test_relative_traversal_value_is_normalized(self):
191+
"""A relative path like '../../outside' combined with the project-root template
192+
must produce a clean normalized absolute path, not '/project/../../outside'."""
193+
var_def = {'result': '{project-root}/{value}'}
194+
result = apply_result_template(
195+
var_def, '../../outside-dir', {'project-root': '/Users/me/myproject'}
196+
)
197+
self.assertEqual(result, '/Users/outside-dir')
198+
199+
def test_relative_one_level_up_is_normalized(self):
200+
var_def = {'result': '{project-root}/{value}'}
201+
result = apply_result_template(
202+
var_def, '../sibling-outputs', {'project-root': '/project/root'}
203+
)
204+
self.assertEqual(result, '/project/sibling-outputs')
205+
206+
def test_normal_relative_value_unchanged(self):
207+
"""Standard in-project relative paths still produce the expected joined path."""
208+
var_def = {'result': '{project-root}/{value}'}
209+
result = apply_result_template(
210+
var_def, '_bmad-output', {'project-root': '/project/root'}
211+
)
212+
self.assertEqual(result, '/project/root/_bmad-output')
213+
150214

151215
class TestLoadModuleYaml(unittest.TestCase):
152216

0 commit comments

Comments
 (0)