Skip to content

Commit b4c4e86

Browse files
fix(integrations): strip UTF-8 BOM when reading agent context files (#2283)
* fix(integrations): strip UTF-8 BOM when reading agent context files * test(integrations): add BOM regression tests for context file read/write * test(workflows): mock shutil.which in tests that assume CLI is absent * test(integrations): remove unused manifest variable in BOM test
1 parent dc057a2 commit b4c4e86

File tree

3 files changed

+55
-6
lines changed

3 files changed

+55
-6
lines changed

src/specify_cli/integrations/base.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ def upsert_context_section(
482482
)
483483

484484
if ctx_path.exists():
485-
content = ctx_path.read_text(encoding="utf-8")
485+
content = ctx_path.read_text(encoding="utf-8-sig")
486486
start_idx = content.find(self.CONTEXT_MARKER_START)
487487
end_idx = content.find(
488488
self.CONTEXT_MARKER_END,
@@ -547,7 +547,7 @@ def remove_context_section(self, project_root: Path) -> bool:
547547
if not ctx_path.exists():
548548
return False
549549

550-
content = ctx_path.read_text(encoding="utf-8")
550+
content = ctx_path.read_text(encoding="utf-8-sig")
551551
start_idx = content.find(self.CONTEXT_MARKER_START)
552552
end_idx = content.find(
553553
self.CONTEXT_MARKER_END,

tests/integrations/test_integration_claude.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Tests for ClaudeIntegration."""
22

3+
import codecs
34
import json
45
import os
56
from unittest.mock import patch
@@ -74,6 +75,46 @@ def test_setup_upserts_context_section(self, tmp_path):
7475
assert "<!-- SPECKIT END -->" in content
7576
assert "read the current plan" in content
7677

78+
def test_upsert_context_section_strips_bom(self, tmp_path):
79+
"""Existing context file with UTF-8 BOM must be cleaned up on upsert."""
80+
integration = get_integration("claude")
81+
ctx_path = tmp_path / integration.context_file
82+
83+
# Write a file that starts with a UTF-8 BOM (as the old PowerShell script did)
84+
bom = codecs.BOM_UTF8
85+
ctx_path.write_bytes(bom + b"# CLAUDE.md\n\nSome existing content.\n")
86+
87+
integration.upsert_context_section(tmp_path)
88+
89+
result = ctx_path.read_bytes()
90+
assert not result.startswith(bom), "BOM must be stripped after upsert"
91+
content = result.decode("utf-8")
92+
assert "<!-- SPECKIT START -->" in content
93+
assert "Some existing content." in content
94+
95+
def test_remove_context_section_strips_bom(self, tmp_path):
96+
"""remove_context_section must clean BOM from context file on Windows-authored files."""
97+
integration = get_integration("claude")
98+
ctx_path = tmp_path / integration.context_file
99+
100+
marker_content = (
101+
"# CLAUDE.md\n\n"
102+
"<!-- SPECKIT START -->\n"
103+
"For additional context about technologies to be used, project structure,\n"
104+
"shell commands, and other important information, read the current plan\n"
105+
"<!-- SPECKIT END -->\n"
106+
)
107+
ctx_path.write_bytes(codecs.BOM_UTF8 + marker_content.encode("utf-8"))
108+
109+
result = integration.remove_context_section(tmp_path)
110+
111+
assert result is True
112+
assert ctx_path.exists(), "File should exist (non-empty content remains)"
113+
remaining = ctx_path.read_bytes()
114+
assert not remaining.startswith(codecs.BOM_UTF8), "BOM must be stripped after remove"
115+
assert b"<!-- SPECKIT" not in remaining
116+
assert b"# CLAUDE.md" in remaining
117+
77118
def test_ai_flag_auto_promotes_and_enables_skills(self, tmp_path):
78119
from typer.testing import CliRunner
79120
from specify_cli import app

tests/test_workflows.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ class TestCommandStep:
400400
"""Test the command step type."""
401401

402402
def test_execute_basic(self):
403+
from unittest.mock import patch
403404
from specify_cli.workflows.steps.command import CommandStep
404405
from specify_cli.workflows.base import StepContext, StepStatus
405406

@@ -413,7 +414,8 @@ def test_execute_basic(self):
413414
"command": "speckit.specify",
414415
"input": {"args": "{{ inputs.name }}"},
415416
}
416-
result = step.execute(config, ctx)
417+
with patch("specify_cli.workflows.steps.command.shutil.which", return_value=None):
418+
result = step.execute(config, ctx)
417419
assert result.status == StepStatus.FAILED
418420
assert result.output["command"] == "speckit.specify"
419421
assert result.output["integration"] == "claude"
@@ -474,6 +476,7 @@ def test_options_merge(self):
474476

475477
def test_dispatch_not_attempted_without_cli(self):
476478
"""When the CLI tool is not installed, step should fail."""
479+
from unittest.mock import patch
477480
from specify_cli.workflows.steps.command import CommandStep
478481
from specify_cli.workflows.base import StepContext, StepStatus
479482

@@ -488,7 +491,8 @@ def test_dispatch_not_attempted_without_cli(self):
488491
"command": "speckit.specify",
489492
"input": {"args": "{{ inputs.name }}"},
490493
}
491-
result = step.execute(config, ctx)
494+
with patch("specify_cli.workflows.steps.command.shutil.which", return_value=None):
495+
result = step.execute(config, ctx)
492496
assert result.status == StepStatus.FAILED
493497
assert result.output["dispatched"] is False
494498
assert result.error is not None
@@ -566,6 +570,7 @@ class TestPromptStep:
566570
"""Test the prompt step type."""
567571

568572
def test_execute_basic(self):
573+
from unittest.mock import patch
569574
from specify_cli.workflows.steps.prompt import PromptStep
570575
from specify_cli.workflows.base import StepContext, StepStatus
571576

@@ -579,7 +584,8 @@ def test_execute_basic(self):
579584
"type": "prompt",
580585
"prompt": "Review {{ inputs.file }} for security issues",
581586
}
582-
result = step.execute(config, ctx)
587+
with patch("specify_cli.workflows.steps.prompt.shutil.which", return_value=None):
588+
result = step.execute(config, ctx)
583589
assert result.status == StepStatus.FAILED
584590
assert result.output["prompt"] == "Review auth.py for security issues"
585591
assert result.output["integration"] == "claude"
@@ -1311,6 +1317,7 @@ def test_load_not_found(self, project_dir):
13111317
engine.load_workflow("nonexistent")
13121318

13131319
def test_execute_simple_workflow(self, project_dir):
1320+
from unittest.mock import patch
13141321
from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition
13151322
from specify_cli.workflows.base import RunStatus
13161323

@@ -1333,7 +1340,8 @@ def test_execute_simple_workflow(self, project_dir):
13331340
"""
13341341
definition = WorkflowDefinition.from_string(yaml_str)
13351342
engine = WorkflowEngine(project_dir)
1336-
state = engine.execute(definition, {"name": "login"})
1343+
with patch("specify_cli.workflows.steps.command.shutil.which", return_value=None):
1344+
state = engine.execute(definition, {"name": "login"})
13371345

13381346
assert state.status == RunStatus.FAILED
13391347
assert "step-one" in state.step_results

0 commit comments

Comments
 (0)