Address Copilot review feedback

SummerOneTwo · SummerOneTwo · commit 8346c7245040 · 2026-04-27T20:13:44.000+08:00
diff --git a/scripts/workflow_guard.py b/scripts/workflow_guard.py
@@ -136,7 +136,7 @@ def pre_tool(payload: dict[str, Any]) -> int:
         "checker_build": "必须先通过 stress_test_run（completed_rounds == total_rounds），再构建 checker。",
         "problem_validate": "必须先通过 stress_test_run（completed_rounds == total_rounds），再进行验证。",
         "problem_generate_tests": "必须先通过 problem_validate（验证通过），才能生成最终测试数据。",
-        "problem_pack_polygon": "必须先生成最终测试数据，并且生成数量 > 0，再进行打包。",
+        "problem_pack_polygon": "必须先生成最终测试数据并通过 problem_verify_tests(passed)，再进行打包。",
     }
 
     tool_input = payload.get("tool_input", {})
@@ -185,6 +185,7 @@ def pre_tool(payload: dict[str, Any]) -> int:
 
     if short_name == "problem_pack_polygon" and not (
         state["tests_generated"] and state.get("generated_test_count", 0) > 0
+        and state.get("tests_verified", False)
     ):
         deny(reasons["problem_pack_polygon"])
         return 0
@@ -210,6 +211,12 @@ def post_tool(payload: dict[str, Any]) -> int:
         save_state(problem_dir, state)
         return 0
 
+    if short_name == "problem_verify_tests" and not success:
+        state = load_state(problem_dir)
+        state["tests_verified"] = False
+        save_state(problem_dir, state)
+        return 0
+
     if not success:
         return 0
 
@@ -245,6 +252,9 @@ def post_tool(payload: dict[str, Any]) -> int:
         generated_tests = data.get("generated_tests", [])
         state["tests_generated"] = bool(generated_tests)
         state["generated_test_count"] = len(generated_tests)
+        state["tests_verified"] = False
+    elif short_name == "problem_verify_tests":
+        state["tests_verified"] = bool(data.get("passed", False))
     elif short_name == "problem_pack_polygon":
         state["packaged"] = True
 
diff --git a/src/autocode_mcp/tools/problem.py b/src/autocode_mcp/tools/problem.py
@@ -493,31 +493,37 @@ def _resolve_tests_dir(
         output_dir: str | None,
     ) -> tuple[str | None, ToolResult | None]:
         """解析并校验测试输出目录，防止清理时误删题目文件或外部目录。"""
-        problem_root = os.path.abspath(problem_dir)
+        problem_root = os.path.realpath(problem_dir)
         raw_output_dir = output_dir or "tests"
         tests_dir = raw_output_dir
         if not os.path.isabs(tests_dir):
             tests_dir = os.path.join(problem_root, tests_dir)
         tests_dir = os.path.abspath(tests_dir)
+        resolved_tests_dir = os.path.realpath(tests_dir)
 
         try:
-            common = os.path.commonpath([problem_root, tests_dir])
+            common = os.path.commonpath([problem_root, resolved_tests_dir])
         except ValueError:
             common = ""
         if os.path.normcase(common) != os.path.normcase(problem_root):
             return None, ToolResult.fail("output_dir must be inside problem_dir")
 
-        if os.path.normcase(tests_dir) == os.path.normcase(problem_root):
+        if os.path.normcase(resolved_tests_dir) == os.path.normcase(problem_root):
             return None, ToolResult.fail("output_dir cannot be the problem_dir root")
 
         reserved_dirs = {"files", "solutions", "statements"}
         for reserved in reserved_dirs:
-            reserved_path = os.path.join(problem_root, reserved)
-            if os.path.normcase(os.path.commonpath([reserved_path, tests_dir])) == os.path.normcase(
-                reserved_path
-            ):
+            reserved_path = os.path.realpath(os.path.join(problem_root, reserved))
+            try:
+                reserved_common = os.path.commonpath([reserved_path, resolved_tests_dir])
+            except ValueError:
+                reserved_common = ""
+            if os.path.normcase(reserved_common) == os.path.normcase(reserved_path):
                 return None, ToolResult.fail(f"output_dir cannot be reserved directory: {reserved}")
 
+        if os.path.exists(tests_dir) and os.path.islink(tests_dir):
+            return None, ToolResult.fail(f"output_dir cannot be a symlink: {tests_dir}")
+
         if os.path.exists(tests_dir) and not os.path.isdir(tests_dir):
             return None, ToolResult.fail(f"output_dir exists and is not a directory: {tests_dir}")
 
diff --git a/src/autocode_mcp/tools/solution.py b/src/autocode_mcp/tools/solution.py
@@ -3,6 +3,7 @@
 """
 
 import os
+import shutil
 from typing import Literal
 
 from ..utils.platform import get_exe_extension
@@ -97,11 +98,15 @@ async def execute(
         exe_ext = get_exe_extension()
         canonical_path = os.path.join(solutions_dir, f"{effective_name}.cpp")
         binary_path = os.path.join(solutions_dir, f"{effective_name}{exe_ext}")
+        standard_source_path = os.path.join(solutions_dir, f"{solution_type}.cpp")
+        standard_binary_path = os.path.join(solutions_dir, f"{solution_type}{exe_ext}")
 
-        # 保存到标准位置（其他工具依赖此路径）
+        # 保存自定义命名文件，并保留 sol.cpp/brute.cpp 供打包和默认流程使用。
         try:
             with open(canonical_path, "w", encoding="utf-8") as f:
                 f.write(resolved.code)
+            if os.path.normcase(canonical_path) != os.path.normcase(standard_source_path):
+                shutil.copy2(canonical_path, standard_source_path)
         except Exception as e:
             return ToolResult.fail(f"Failed to save code: {str(e)}")
 
@@ -119,11 +124,18 @@ async def execute(
             )
 
         binary_size = os.path.getsize(binary_path) if os.path.exists(binary_path) else 0
+        if os.path.normcase(binary_path) != os.path.normcase(standard_binary_path):
+            try:
+                shutil.copy2(binary_path, standard_binary_path)
+            except Exception as e:
+                return ToolResult.fail(f"Failed to save standard binary: {str(e)}")
 
         return ToolResult.ok(
             source_path=compile_source,
             canonical_path=canonical_path,
+            standard_source_path=standard_source_path,
             binary_path=binary_path,
+            standard_binary_path=standard_binary_path,
             binary_size=binary_size,
             compile_log=result.stderr,
             effective_name=effective_name,
diff --git a/src/autocode_mcp/tools/test_verify.py b/src/autocode_mcp/tools/test_verify.py
@@ -182,12 +182,13 @@ def _check_file_count(self, tests_dir: str) -> dict:
 
         non_numeric = [f for f in in_files if not Path(f).stem.isdigit()]
         numeric_indices = sorted(int(Path(f).stem) for f in in_files if Path(f).stem.isdigit())
-        expected_indices = list(range(1, len(numeric_indices) + 1))
+        numeric_index_set = set(numeric_indices)
+        expected_indices = list(range(1, max(numeric_indices) + 1)) if numeric_indices else []
         missing_indices = [
-            idx for idx in expected_indices if idx not in numeric_indices
-        ] if numeric_indices else []
+            idx for idx in expected_indices if idx not in numeric_index_set
+        ]
         duplicate_indices = sorted(
-            idx for idx in set(numeric_indices) if numeric_indices.count(idx) > 1
+            idx for idx in numeric_index_set if numeric_indices.count(idx) > 1
         )
 
         passed = (
diff --git a/tests/test_tools/test_problem.py b/tests/test_tools/test_problem.py
@@ -324,6 +324,30 @@ async def test_problem_generate_tests_rejects_unsafe_output_dir():
         assert "output_dir must be inside problem_dir" in result.error
 
 
+@pytest.mark.asyncio
+async def test_problem_generate_tests_rejects_symlinked_output_dir():
+    """测试拒绝指向题目目录外部的符号链接输出目录。"""
+    tool = ProblemGenerateTestsTool()
+
+    with tempfile.TemporaryDirectory() as tmpdir:
+        problem_dir = os.path.join(tmpdir, "symlink_output")
+        outside_dir = os.path.join(tmpdir, "outside_tests")
+        link_dir = os.path.join(problem_dir, "tests_link")
+        os.makedirs(os.path.join(problem_dir, "files"))
+        os.makedirs(os.path.join(problem_dir, "solutions"))
+        os.makedirs(outside_dir)
+
+        try:
+            os.symlink(outside_dir, link_dir, target_is_directory=True)
+        except (OSError, NotImplementedError):
+            pytest.skip("symlink creation is not available")
+
+        result = await tool.execute(problem_dir=problem_dir, output_dir="tests_link")
+
+        assert not result.success
+        assert "output_dir must be inside problem_dir" in result.error
+
+
 def test_problem_generate_tests_clear_only_generated_files():
     """测试清理输出目录时只删除旧的 .in/.ans 文件。"""
     tool = ProblemGenerateTestsTool()
@@ -406,6 +430,23 @@ def test_problem_verify_tests_file_count_requires_contiguous_numeric_names():
         assert result["missing_indices"] == [2]
 
 
+def test_problem_verify_tests_file_count_reports_large_gaps():
+    """测试跳到大编号时会报告完整缺失区间。"""
+    tool = ProblemVerifyTestsTool()
+
+    with tempfile.TemporaryDirectory() as tmpdir:
+        for name in ["01.in", "01.ans", "100.in", "100.ans"]:
+            with open(os.path.join(tmpdir, name), "w", encoding="utf-8") as f:
+                f.write("x\n")
+
+        result = tool._check_file_count(tmpdir)
+
+        assert not result["passed"]
+        assert result["missing_indices"][0] == 2
+        assert result["missing_indices"][-1] == 99
+        assert len(result["missing_indices"]) == 98
+
+
 @pytest.mark.asyncio
 async def test_problem_pack_polygon_dynamic_test_count():
     """测试 Polygon 打包使用动态 test-count。"""
diff --git a/tests/test_tools/test_solution.py b/tests/test_tools/test_solution.py
@@ -277,6 +277,34 @@ async def test_solution_build():
         assert os.path.exists(result.data["binary_path"])
 
 
+@pytest.mark.asyncio
+async def test_solution_build_custom_name_keeps_standard_files(monkeypatch):
+    """测试自定义命名构建时仍保留 sol.cpp/sol 可供默认流程使用。"""
+    tool = SolutionBuildTool()
+
+    async def fake_build(source_path, binary_path, compiler="g++", include_dirs=None):
+        with open(binary_path, "w", encoding="utf-8") as f:
+            f.write("binary")
+        return CompileResult(success=True, binary_path=binary_path)
+
+    monkeypatch.setattr(tool, "build", fake_build)
+
+    with tempfile.TemporaryDirectory() as tmpdir:
+        result = await tool.execute(
+            problem_dir=tmpdir,
+            solution_type="sol",
+            name="accepted",
+            code=SIMPLE_CPP,
+        )
+
+        assert result.success
+        assert os.path.exists(os.path.join(tmpdir, "solutions", "accepted.cpp"))
+        assert os.path.exists(os.path.join(tmpdir, "solutions", "sol.cpp"))
+        assert os.path.exists(result.data["binary_path"])
+        assert os.path.exists(result.data["standard_binary_path"])
+        assert result.data["effective_name"] == "accepted"
+
+
 @pytest.mark.asyncio
 async def test_solution_build_brute():
     """测试暴力解法构建。"""
diff --git a/tests/test_workflow_guard.py b/tests/test_workflow_guard.py
@@ -72,3 +72,92 @@ def test_post_tool_marks_stress_passed(tmp_path):
 
     assert exit_code == 0
     assert state["stress_passed"] is True
+
+
+def test_pre_tool_denies_pack_before_tests_verified(tmp_path, capsys):
+    module = load_module()
+    problem_dir = tmp_path / "problem"
+    (problem_dir / "files").mkdir(parents=True)
+    (problem_dir / "solutions").mkdir(parents=True)
+    state = {
+        "problem_dir": str(problem_dir),
+        "created": True,
+        "sol_built": True,
+        "brute_built": True,
+        "validator_ready": True,
+        "validator_accuracy": 1.0,
+        "generator_built": True,
+        "stress_passed": True,
+        "checker_ready": False,
+        "validation_passed": True,
+        "tests_generated": True,
+        "generated_test_count": 3,
+        "tests_verified": False,
+        "packaged": False,
+    }
+    module.save_state(str(problem_dir), state)
+
+    payload = {
+        "tool_name": "mcp__autocode__problem_pack_polygon",
+        "tool_input": {"problem_dir": str(problem_dir)},
+    }
+
+    exit_code = module.pre_tool(payload)
+    captured = capsys.readouterr().out
+
+    assert exit_code == 0
+    parsed = json.loads(captured)
+    assert parsed["hookSpecificOutput"]["permissionDecision"] == "deny"
+    assert "problem_verify_tests" in parsed["hookSpecificOutput"]["permissionDecisionReason"]
+
+
+def test_post_tool_marks_tests_verified(tmp_path):
+    module = load_module()
+    problem_dir = tmp_path / "problem"
+    (problem_dir / "files").mkdir(parents=True)
+    (problem_dir / "solutions").mkdir(parents=True)
+
+    payload = {
+        "tool_name": "mcp__autocode__problem_verify_tests",
+        "tool_input": {"problem_dir": str(problem_dir)},
+        "tool_response": {
+            "structuredContent": {
+                "success": True,
+                "data": {"passed": True},
+            }
+        },
+    }
+
+    exit_code = module.post_tool(payload)
+    state = module.load_state(str(problem_dir))
+
+    assert exit_code == 0
+    assert state["tests_verified"] is True
+
+
+def test_post_tool_clears_tests_verified_after_regeneration(tmp_path):
+    module = load_module()
+    problem_dir = tmp_path / "problem"
+    (problem_dir / "files").mkdir(parents=True)
+    (problem_dir / "solutions").mkdir(parents=True)
+    state = module.infer_state(str(problem_dir))
+    state["tests_verified"] = True
+    module.save_state(str(problem_dir), state)
+
+    payload = {
+        "tool_name": "mcp__autocode__problem_generate_tests",
+        "tool_input": {"problem_dir": str(problem_dir)},
+        "tool_response": {
+            "structuredContent": {
+                "success": True,
+                "data": {"generated_tests": [1, 2]},
+            }
+        },
+    }
+
+    exit_code = module.post_tool(payload)
+    state = module.load_state(str(problem_dir))
+
+    assert exit_code == 0
+    assert state["tests_generated"] is True
+    assert state["tests_verified"] is False
