fix: address coderabbit diagnostics findings

Author: bma-d

skills/bmad-story-automator/data/tmux-commands.md

@@ -38,28 +38,21 @@ project_hash=$("$script" tmux-wrapper project-hash)
 **Generate full session name:**
 ```bash
 script="$(printf "%s" "{project_root}/{installed-skill-root}/bmad-story-automator/scripts/story-automator")"
-project_slug=$("$script" tmux-wrapper project-slug)
-project_hash=$("$script" tmux-wrapper project-hash)
-timestamp=$(date +%y%m%d-%H%M%S)  # Returns "260114-223045"
-session_name="sa-${project_slug}-${project_hash}-${timestamp}-e{epic}-s{story_suffix}-{step}"
+session_name=$("$script" tmux-wrapper name "{step}" "{epic}" "{story_id}")
 ```
 
 ### Listing/Killing Project-Specific Sessions
 
 **List only current project's sessions:**
 ```bash
 script="$(printf "%s" "{project_root}/{installed-skill-root}/bmad-story-automator/scripts/story-automator")"
-project_slug=$("$script" tmux-wrapper project-slug)
-project_hash=$("$script" tmux-wrapper project-hash)
-tmux list-sessions 2>/dev/null | grep "^sa-${project_slug}-${project_hash}-"
+"$script" tmux-wrapper list --project-only
 ```
 
 **Kill only current project's sessions:**
 ```bash
 script="$(printf "%s" "{project_root}/{installed-skill-root}/bmad-story-automator/scripts/story-automator")"
-project_slug=$("$script" tmux-wrapper project-slug)
-project_hash=$("$script" tmux-wrapper project-hash)
-tmux list-sessions -F '#{session_name}' 2>/dev/null | grep "^sa-${project_slug}-${project_hash}-" | xargs -I {} tmux kill-session -t {}
+"$script" tmux-wrapper kill-all --project-only
 ```
 
 ### No Dots in Session Names

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

@@ -99,6 +99,6 @@ def _load_presets_or_report(file_path: str) -> dict | None:
     except json.JSONDecodeError:
         print_json({"ok": False, "error": "invalid_presets_json"})
         return None
-    except OSError as exc:
+    except (OSError, UnicodeDecodeError) as exc:
         print_json({"ok": False, "error": "presets_file_error", "reason": str(exc)})
         return None

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

@@ -301,15 +301,16 @@ def _state_update(args: list[str]) -> int:
         pending_status = value
         final_status = value
     updated: list[str] = []
+    frontmatter, body = _split_frontmatter(text)
     for key, value in updates:
-        replaced, count = re.subn(rf"(?m)^{re.escape(key)}:.*$", lambda m, k=key, v=value: f"{k}: {v}", text)
+        replaced, count = re.subn(rf"(?m)^{re.escape(key)}:.*$", lambda m, k=key, v=value: f"{k}: {v}", frontmatter)
         if count:
-            text = replaced
+            frontmatter = replaced
             updated.append(key)
     if not updated:
         print_json({"ok": False, "error": "keys_not_found", "updated": []})
         return 1
-    Path(args[0]).write_text(text, encoding="utf-8")
+    Path(args[0]).write_text(frontmatter + body, encoding="utf-8")
     if final_status:
         emit_state_transition(args[0], result="applied", new_status=final_status)
     event_fields = [key for key in updated if key in {"epic", "currentStory", "currentStep", "lastUpdated"}]
@@ -319,6 +320,15 @@ def _state_update(args: list[str]) -> int:
     return 0
 
 
+def _split_frontmatter(text: str) -> tuple[str, str]:
+    if not text.startswith("---"):
+        return text, ""
+    parts = text.split("---", 2)
+    if len(parts) < 3:
+        return text, ""
+    return f"{parts[0]}---{parts[1]}---", parts[2]
+
+
 def _escalate(args: list[str]) -> int:
     trigger = args[0] if args else ""
     context = args[1] if len(args) > 1 else ""

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

@@ -7,9 +7,10 @@
 
 def extract_agent_config_frontmatter(frontmatter: str) -> dict[str, object]:
     for index, raw_line in enumerate(frontmatter.splitlines()):
-        stripped = raw_line.strip()
-        if stripped.startswith("agentConfig:"):
+        if raw_line.startswith("agentConfig:"):
             return _extract_agent_config_block(frontmatter.splitlines(), index)
+        if raw_line.strip().startswith("agentConfig:"):
+            raise ValueError("agentConfig must be a top-level frontmatter key")
     return {}
 
 
@@ -39,6 +40,8 @@ def _parse_indented_map(lines: list[str]) -> dict[str, object]:
         line = _strip_inline_yaml_comment(raw_line.rstrip())
         if not line.strip():
             continue
+        if "\t" in line:
+            raise ValueError("agentConfig block must use spaces, not tabs")
         indent = len(line) - len(line.lstrip(" "))
         if indent % 2 != 0:
             raise ValueError("agentConfig indentation must use two-space levels")

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

@@ -19,9 +19,9 @@
     r"(?i)\b(authorization|credential|password|secret|token|api[_-]?key|access[_-]?key)\b\s*[:=]\s*(?:(?:bearer|basic|token)\s+)?[^\s,;]+"
 )
 ABSOLUTE_PATH_WITH_EXT_RE = re.compile(
-    r"(?<![\w.-])/(?:[^,\n;:]+/)+[^,\n;:]*?\.[A-Za-z0-9][A-Za-z0-9._-]*(?=$|[\s,;:)\]}\"'])"
+    r"(?<![\w.-])(?:/(?:[^,\n;:]+/)+[^,\n;:]*?|[A-Za-z]:[\\/](?:[^,\n;:]+[\\/])+[^,\n;:]*?)\.[A-Za-z0-9][A-Za-z0-9._-]*(?=$|[\s,;:)\]}\"'])"
 )
-ABSOLUTE_PATH_RE = re.compile(r"(?<![\w.-])(?:/[^\s,;:]+)+")
+ABSOLUTE_PATH_RE = re.compile(r"(?<![\w.-])(?:/[^\s,;:]+|[A-Za-z]:[\\/][^\s,;:]+)+")
 
 
 @dataclass(frozen=True)
@@ -164,5 +164,5 @@ def _redact_string(value: str) -> str:
 
 def _path_placeholder(match: re.Match[str]) -> str:
     path = match.group(0)
-    name = Path(path).name
+    name = path.replace("\\", "/").rstrip("/").rsplit("/", 1)[-1]
     return f"<path:{name}>" if name else "<path>"

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

@@ -145,7 +145,7 @@ def parse_state_update_argument(raw: str) -> tuple[str, str] | dict[str, Any]:
     key, value = raw.split("=", 1)
     if not key.strip():
         return state_update_argument_error_payload(raw)
-    return key, value
+    return key.strip(), value
 
 
 def state_validation_payload(issues: list[DiagnosticIssue]) -> dict[str, Any]:

tests/test_cli_contracts.py

@@ -162,6 +162,14 @@ def test_malformed_presets_file_returns_stable_error(self) -> None:
         self.assertEqual(code, 1)
         self.assertEqual(payload["error"], "invalid_presets_json")
 
+    def test_presets_decode_error_returns_stable_error(self) -> None:
+        self.presets.write_bytes(b"\xff")
+
+        code, payload = self._agent(["list", "--file", str(self.presets)])
+
+        self.assertEqual(code, 1)
+        self.assertEqual(payload["error"], "presets_file_error")
+
     def _agent(self, args: list[str]) -> tuple[int, dict[str, object]]:
         stdout = io.StringIO()
         with redirect_stdout(stdout):

tests/test_diagnostics.py

@@ -131,6 +131,13 @@ def test_redact_actual_masks_absolute_paths_with_spaces(self) -> None:
         self.assertNotIn("My Project", redacted)
         self.assertNotIn("private/state.md", redacted)
 
+    def test_redact_actual_masks_windows_absolute_paths(self) -> None:
+        redacted = redact_actual(r"C:\Users\joon\private\state.md token=abc123")
+
+        self.assertEqual(redacted, "<path:state.md> token=<redacted>")
+        self.assertNotIn(r"C:\Users", redacted)
+        self.assertNotIn(r"private\state.md", redacted)
+
     def test_redact_actual_limits_nested_collections(self) -> None:
         payload = redact_actual({"values": list(range(10)), **{f"k{i}": i for i in range(10)}})
 

tests/test_retro_agent.py

@@ -330,7 +330,9 @@ def test_retro_agent_rejects_invalid_nested_complexity_override_frontmatter(self
             "---\nagentConfig:\n  complexityOverrides:\n    medium:\n     retro:\n        primary: \"codex\"\n---\n",
             "---\nagentConfig:\n  defaultPrimary: \"claude\"\n    complexityOverrides:\n      medium:\n        retro:\n          primary: \"codex\"\n---\n",
             "---\nagentConfig: bad\n  complexityOverrides:\n    medium:\n      retro:\n        primary: \"codex\"\n---\n",
+            "---\n  agentConfig: {defaultPrimary: codex}\n---\n",
             "---\nagentConfig:\n\tdefaultPrimary: \"claude\"\n\tcomplexityOverrides:\n\t  medium:\n\t    retro:\n\t      primary: \"codex\"\n---\n",
+            "---\nagentConfig:\n  \tdefaultPrimary: \"claude\"\n---\n",
             "---\nagentConfig:\ncomplexityOverrides:\n  medium:\n    retro:\n      primary: \"codex\"\n---\n",
         )
         for index, content in enumerate(cases):

tests/test_state_validation.py

@@ -60,6 +60,7 @@ def test_runtime_command_config_rejects_whitespace_only_command(self) -> None:
         self.assertFalse(has_runtime_command_config({"aiCommand": ["", "  "]}, ""))
         self.assertTrue(has_runtime_command_config({"aiCommand": ["  claude  "]}, ""))
         self.assertTrue(has_runtime_command_config({"aiCommand": "   "}, 'agentConfig:\n  defaultPrimary: "codex"\n'))
+        self.assertFalse(has_runtime_command_config({"aiCommand": "   "}, '  agentConfig:\n    defaultPrimary: "codex"\n'))
         self.assertFalse(has_runtime_command_config({"aiCommand": "   "}, "agentConfig:\n  defaultPrimary:\n"))
         self.assertFalse(has_runtime_command_config({"aiCommand": "   "}, "agentConfig:\n  complexityOverrides:\n    - medium:\n"))
 
@@ -247,6 +248,31 @@ def test_state_update_still_allows_non_status_updates(self) -> None:
         self.assertEqual(payload, {"ok": True, "updated": ["aiCommand"]})
         self.assertIn("aiCommand: claude --resume", state_file.read_text(encoding="utf-8"))
 
+    def test_state_update_only_rewrites_frontmatter(self) -> None:
+        state_file = self._build_state_config(status="COMPLETE")
+        text = state_file.read_text(encoding="utf-8").replace("currentStep: null\n", "currentStep: step-old\n", 1)
+        state_file.write_text(text + "\nstatus: body-marker\ncurrentStep: body-step\n", encoding="utf-8")
+
+        code, payload = self._state_update_args(state_file, ["--set", "status=COMPLETE", "--set", "currentStep=step-next"])
+
+        self.assertEqual(code, 0)
+        self.assertEqual(payload, {"ok": True, "updated": ["status", "currentStep"]})
+        text = state_file.read_text(encoding="utf-8")
+        frontmatter = text.split("---", 2)[1]
+        body = text.split("---", 2)[2]
+        self.assertIn("status: COMPLETE", frontmatter)
+        self.assertIn("currentStep: step-next", frontmatter)
+        self.assertIn("status: body-marker", body)
+        self.assertIn("currentStep: body-step", body)
+
+    def test_state_update_strips_set_key_whitespace(self) -> None:
+        state_file = self._build_state_config(status="READY")
+
+        code, payload = self._state_update(state_file, " status=COMPLETE")
+
+        self.assertEqual(code, 1)
+        self.assertEqual(payload["error"], "invalid_status_transition")
+
     def _validate_state(self, state_file: Path) -> dict[str, object]:
         stdout = io.StringIO()
         with patch_env(self.project_root), redirect_stdout(stdout):