Skip to content
This repository was archived by the owner on Jul 13, 2026. It is now read-only.

Commit 6d63159

Browse files
committed
fix: address augment validation findings
1 parent 87decbd commit 6d63159

10 files changed

Lines changed: 88 additions & 16 deletions

File tree

skills/bmad-story-automator/src/story_automator/commands/agent_config_cmd.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,6 @@ def _load_presets_or_report(file_path: str) -> dict | None:
102102
except (OSError, UnicodeDecodeError) as exc:
103103
print_json({"ok": False, "error": "presets_file_error", "reason": str(exc)})
104104
return None
105+
except ValueError:
106+
print_json({"ok": False, "error": "invalid_presets_json"})
107+
return None

skills/bmad-story-automator/src/story_automator/commands/orchestrator_state.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ def _replace_frontmatter_values(frontmatter: str, updates: list[tuple[str, str]]
7676

7777
def _split_frontmatter(text: str) -> tuple[str, str]:
7878
if not text.startswith("---"):
79-
return text, ""
79+
return "", text
8080
parts = text.split("---", 2)
8181
if len(parts) < 3:
82-
return text, ""
82+
return "", text
8383
return f"{parts[0]}---{parts[1]}---", parts[2]

skills/bmad-story-automator/src/story_automator/commands/tmux.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,10 +363,10 @@ def cmd_monitor_session(args: list[str]) -> int:
363363
start = time.time()
364364
last_done = 0
365365
last_total = 0
366+
session_state_issue = monitor_session_state_issue(session, project_root) if json_output else None
366367
for _poll in range(1, max_polls + 1):
367368
if time.time() - start >= timeout_minutes * 60:
368369
return emit_monitor_result(json_output, "timeout", last_done, last_total, "", f"exceeded_{timeout_minutes}m")
369-
pre_status_issue = monitor_session_state_issue(session, project_root) if json_output else None
370370
status = session_status(session, full=False, codex=agent == "codex", project_root=project_root, mode=runtime_mode())
371371
if int(status["todos_done"]) or int(status["todos_total"]):
372372
last_done = int(status["todos_done"])
@@ -418,7 +418,7 @@ def cmd_monitor_session(args: list[str]) -> int:
418418
output = session_status(session, full=True, codex=agent == "codex", project_root=project_root, mode=runtime_mode())["active_task"]
419419
return emit_monitor_result(json_output, "stuck", 0, 0, str(output), "never_active")
420420
if state == "not_found":
421-
issue = pre_status_issue or monitor_session_state_issue(session, project_root)
421+
issue = session_state_issue or (monitor_session_state_issue(session, project_root) if json_output else None)
422422
return emit_monitor_result(json_output, "not_found", last_done, last_total, "", "session_gone", structured_issue=issue)
423423
time.sleep(min(180 if agent == "codex" else 120, max(5, int(status["wait_estimate"]))))
424424
output = session_status(session, full=True, codex=agent == "codex", project_root=project_root, mode=runtime_mode())["active_task"]

skills/bmad-story-automator/src/story_automator/core/agent_config.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ def load_presets_file(path: str | Path) -> dict[str, Any]:
4040
if not file_exists(preset_path):
4141
return {"version": "1.0.0", "presets": []}
4242
data = json.loads(read_text(preset_path))
43+
if not isinstance(data, dict):
44+
raise ValueError("presets file must be an object")
4345
data.setdefault("version", "1.0.0")
4446
data.setdefault("presets", [])
47+
if not isinstance(data["presets"], list):
48+
raise ValueError("presets file presets must be an array")
4549
return data
4650

4751

@@ -56,7 +60,7 @@ def parse_agent_config_json(raw: str) -> AgentConfigResolved:
5660
raise ValueError("agentConfig must be an object")
5761
config = AgentConfigResolved()
5862
if "agentConfig" in data and data.get("agentConfig") not in ("", None):
59-
raise ValueError("agentConfig must be an object")
63+
raise ValueError("unexpected nested agentConfig key; pass the inner config object directly")
6064
config.default_primary = data.get("defaultPrimary") or data.get("primary") or "auto"
6165
if "defaultFallback" in data:
6266
fallback_raw = data.get("defaultFallback")

skills/bmad-story-automator/src/story_automator/core/agent_plan.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,9 @@ def validate_complexity_payload(payload: object) -> list[DiagnosticIssue]:
3636
story_id = story.get("storyId")
3737
if not isinstance(story_id, str) or not story_id.strip():
3838
issues.append(_issue("missing_field", f"{field}.storyId", "non-empty string", story_id, "Complexity storyId must be a non-empty string"))
39-
complexity = story.get("complexity")
40-
if complexity is None:
41-
complexity = {}
42-
elif not isinstance(complexity, dict):
43-
issues.append(_issue("invalid_type", f"{field}.complexity", "object", complexity, "Complexity must be an object"))
39+
complexity, issue = _story_complexity(story, field)
40+
if issue:
41+
issues.append(issue)
4442
continue
4543
level = str(complexity.get("level") or "medium").strip().lower()
4644
if level not in COMPLEXITY_LEVELS:
@@ -132,8 +130,8 @@ def build_agents_file(
132130
raise AgentPlanInputError("complexity-file", ValueError(message)) from None
133131

134132
stories = []
135-
for story in complexity_payload.get("stories", []):
136-
level = _story_complexity_level(story)
133+
for index, story in enumerate(complexity_payload.get("stories", [])):
134+
level = _story_complexity_level(story, f"stories[{index}]")
137135
stories.append({"storyId": story.get("storyId"), "title": str(story.get("title") or ""), "complexity": level, "tasks": _tasks_for(config, level)})
138136
try:
139137
epic = find_frontmatter_value(state_file, "epic")
@@ -198,12 +196,19 @@ def _load_agents_plan_payload(path: str) -> tuple[dict[str, Any], list[Diagnosti
198196
return payload, []
199197

200198

201-
def _story_complexity_level(story: dict[str, Any]) -> str:
199+
def _story_complexity(story: dict[str, Any], field: str) -> tuple[dict[str, Any], DiagnosticIssue | None]:
202200
complexity = story.get("complexity")
203201
if complexity is None:
204-
return "medium"
202+
return {}, None
205203
if not isinstance(complexity, dict):
206-
raise AgentPlanInputError("complexity-file", ValueError("Complexity must be an object"))
204+
return {}, _issue("invalid_type", f"{field}.complexity", "object", complexity, "Complexity must be an object")
205+
return complexity, None
206+
207+
208+
def _story_complexity_level(story: dict[str, Any], field: str) -> str:
209+
complexity, issue = _story_complexity(story, field)
210+
if issue:
211+
raise AgentPlanInputError("complexity-file", ValueError(legacy_issue_message(issue)))
207212
return str(complexity.get("level") or "medium").strip().lower() or "medium"
208213

209214

skills/bmad-story-automator/src/story_automator/core/state_validation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def parse_state_update_argument(raw: str) -> tuple[str, str] | dict[str, Any]:
145145
key, value = raw.split("=", 1)
146146
if not key.strip():
147147
return state_update_argument_error_payload(raw)
148-
return key.strip(), value
148+
return key.strip(), value.strip()
149149

150150

151151
def state_validation_payload(issues: list[DiagnosticIssue]) -> dict[str, Any]:

tests/test_agent_config_model.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ def test_agent_cli_treats_empty_model_as_absent(self) -> None:
6060

6161

6262
class CoreAgentConfigModelTests(unittest.TestCase):
63+
def test_parse_agent_config_json_rejects_nested_agent_config_with_clear_message(self) -> None:
64+
with self.assertRaisesRegex(ValueError, "unexpected nested agentConfig key"):
65+
parse_agent_config_json(json.dumps({"agentConfig": {"defaultPrimary": "codex"}}))
66+
6367
def test_per_task_model_is_resolved(self) -> None:
6468
config = parse_agent_config_json(
6569
json.dumps(

tests/test_cli_contracts.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,16 @@ def test_presets_decode_error_returns_stable_error(self) -> None:
170170
self.assertEqual(code, 1)
171171
self.assertEqual(payload["error"], "presets_file_error")
172172

173+
def test_presets_wrong_shape_returns_stable_error(self) -> None:
174+
for payload_text in ("[]", '"bad"', '{"presets": {}}'):
175+
with self.subTest(payload=payload_text):
176+
self.presets.write_text(payload_text, encoding="utf-8")
177+
178+
code, payload = self._agent(["list", "--file", str(self.presets)])
179+
180+
self.assertEqual(code, 1)
181+
self.assertEqual(payload["error"], "invalid_presets_json")
182+
173183
def _agent(self, args: list[str]) -> tuple[int, dict[str, object]]:
174184
stdout = io.StringIO()
175185
with redirect_stdout(stdout):

tests/test_state_validation.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,26 @@ def test_state_update_only_rewrites_frontmatter(self) -> None:
265265
self.assertIn("status: body-marker", body)
266266
self.assertIn("currentStep: body-step", body)
267267

268+
def test_state_update_rejects_file_without_frontmatter_without_rewriting_body(self) -> None:
269+
state_file = self.project_root / "body-only.md"
270+
state_file.write_text("body\nstatus: body-marker\n", encoding="utf-8")
271+
272+
code, payload = self._state_update(state_file, "status=READY")
273+
274+
self.assertEqual(code, 1)
275+
self.assertEqual(payload, {"ok": False, "error": "keys_not_found", "updated": []})
276+
self.assertEqual(state_file.read_text(encoding="utf-8"), "body\nstatus: body-marker\n")
277+
278+
def test_state_update_rejects_unterminated_frontmatter_without_rewriting_body(self) -> None:
279+
state_file = self.project_root / "unterminated.md"
280+
state_file.write_text("---\nstatus: body-marker\n", encoding="utf-8")
281+
282+
code, payload = self._state_update(state_file, "status=READY")
283+
284+
self.assertEqual(code, 1)
285+
self.assertEqual(payload, {"ok": False, "error": "keys_not_found", "updated": []})
286+
self.assertEqual(state_file.read_text(encoding="utf-8"), "---\nstatus: body-marker\n")
287+
268288
def test_state_update_strips_set_key_whitespace(self) -> None:
269289
state_file = self._build_state_config(status="READY")
270290

@@ -273,6 +293,15 @@ def test_state_update_strips_set_key_whitespace(self) -> None:
273293
self.assertEqual(code, 1)
274294
self.assertEqual(payload["error"], "invalid_status_transition")
275295

296+
def test_state_update_strips_set_value_whitespace(self) -> None:
297+
state_file = self._build_state_config(status="READY")
298+
299+
code, payload = self._state_update(state_file, " status = IN_PROGRESS")
300+
301+
self.assertEqual(code, 0)
302+
self.assertEqual(payload, {"ok": True, "updated": ["status"]})
303+
self.assertIn("status: IN_PROGRESS", state_file.read_text(encoding="utf-8"))
304+
276305
def _validate_state(self, state_file: Path) -> dict[str, object]:
277306
stdout = io.StringIO()
278307
with patch_env(self.project_root), redirect_stdout(stdout):

tests/test_success_verifiers.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,23 @@ def test_monitor_session_json_reports_malformed_session_state_when_session_gone(
287287
self.assertEqual(payload["final_state"], "not_found")
288288
self.assertEqual(payload["structuredIssues"][0]["type"], "session_state.invalid_json")
289289

290+
def test_monitor_session_checks_session_state_issue_only_when_session_is_gone(self) -> None:
291+
session = "sa-test-session"
292+
statuses = [
293+
{"active_task": "", "todos_done": 0, "todos_total": 0, "wait_estimate": 0, "session_state": "running"},
294+
{"active_task": "", "todos_done": 0, "todos_total": 0, "wait_estimate": 0, "session_state": "running"},
295+
{"active_task": "", "todos_done": 0, "todos_total": 0, "wait_estimate": 0, "session_state": "not_found"},
296+
]
297+
stdout = io.StringIO()
298+
with patch_env(self.project_root), patch("story_automator.commands.tmux.time.sleep"), patch(
299+
"story_automator.commands.tmux.session_status",
300+
side_effect=statuses,
301+
), patch("story_automator.commands.tmux.monitor_session_state_issue", return_value=None) as state_issue_mock, redirect_stdout(stdout):
302+
code = cmd_monitor_session([session, "--json", "--max-polls", "3"])
303+
304+
self.assertEqual(code, 0)
305+
self.assertEqual(state_issue_mock.call_count, 2)
306+
290307
def test_monitor_session_csv_does_not_include_structured_issues(self) -> None:
291308
session = "sa-test-session"
292309
paths = session_paths(session, self.project_root)

0 commit comments

Comments
 (0)