Skip to content

Commit 2512f53

Browse files
committed
Resolve symlinks in Glob/Grep paths and plan file for cross-platform correctness
- glob.py/grep.py: use realpath instead of abspath so git pathspecs are computed from the canonical path (macOS /var -> /private/var); previously git rejected symlinked repo paths as "outside repository" - planmode.py: store the plan file as realpath so the plan-mode write guard matches tool targets (plan file was blocked on macOS) - Add deterministic regression tests for symlinked search paths and symlinked plan temp dirs (fail on the old code)
1 parent 8fe0855 commit 2512f53

8 files changed

Lines changed: 86 additions & 21 deletions

File tree

‎python_agent_harness/planmode.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,11 @@ def ensure_plan_file(self) -> str:
6464
os.makedirs(os.path.dirname(path), exist_ok=True)
6565
if not os.path.exists(path):
6666
Path(path).write_text("", encoding="utf-8")
67-
self.plan_file = path
68-
return path
67+
# Store the canonical path so the plan-mode write guard (which
68+
# compares against realpath'd tool targets) matches — on macOS
69+
# TMPDIR (/var) is a symlink to /private/var.
70+
self.plan_file = os.path.realpath(path)
71+
return self.plan_file
6972

7073
def cleanup_plan_file(self) -> None:
7174
if not self.plan_file:

‎python_agent_harness/tools/glob.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ def run(self, args: dict, ctx: ToolContext) -> str:
7070
return f"Error: path {path} is not readable"
7171
else:
7272
path = ctx.cwd
73-
base = os.path.abspath(path) # directory-file-name + expand-file-name
73+
# realpath (not abspath): _git_root resolves symlinks (macOS /var ->
74+
# /private/var), so the base must be canonical or relpath produces a
75+
# pathspec git rejects as "outside repository".
76+
base = os.path.realpath(path)
7477
depth = args.get("depth")
7578

7679
git_root = _git_root(base)

‎python_agent_harness/tools/grep.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class Grep(Tool):
4141

4242
def run(self, args: dict, ctx: ToolContext) -> str:
4343
regex = args["regex"]
44-
path = os.path.abspath(args["path"])
44+
path = os.path.realpath(args["path"])
4545
if not os.path.isdir(path) and not os.path.isfile(path):
4646
return f"Error: path {args['path']} is not readable"
4747
glob = args.get("glob")

‎tests/test_agents_md.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def test_finds_agents_md_in_project_root(self):
4646
with tempfile.TemporaryDirectory() as d:
4747
(Path(d) / "AGENTS.md").write_text("# Rules\n", encoding="utf-8")
4848
result = find_agents_md_files(d)
49-
self.assertEqual([Path(p) for p in result], [Path(d) / "AGENTS.md"])
49+
self.assertEqual([Path(p) for p in result], [(Path(d) / "AGENTS.md").resolve()])
5050

5151
@unittest.skipUnless(shutil.which("git"), "git not available")
5252
def test_finds_agents_md_in_ancestor(self):
@@ -57,7 +57,7 @@ def test_finds_agents_md_in_ancestor(self):
5757
sub.mkdir(parents=True)
5858
(Path(d) / "AGENTS.md").write_text("# Root rules\n", encoding="utf-8")
5959
result = find_agents_md_files(str(sub))
60-
self.assertEqual([Path(p) for p in result], [Path(d) / "AGENTS.md"])
60+
self.assertEqual([Path(p) for p in result], [(Path(d) / "AGENTS.md").resolve()])
6161

6262
@unittest.skipUnless(shutil.which("git"), "git not available")
6363
def test_stacks_every_ancestor_nearest_first(self):
@@ -76,7 +76,7 @@ def test_stacks_every_ancestor_nearest_first(self):
7676
result = find_agents_md_files(str(sub))
7777
self.assertEqual(
7878
[Path(p) for p in result],
79-
[pkg / "AGENTS.md", Path(d) / "AGENTS.md"],
79+
[(pkg / "AGENTS.md").resolve(), (Path(d) / "AGENTS.md").resolve()],
8080
)
8181

8282
@unittest.skipUnless(shutil.which("git"), "git not available")
@@ -92,7 +92,7 @@ def test_stops_at_git_root(self):
9292
# above the git root: must NOT be picked up
9393
(Path(outer) / "AGENTS.md").write_text("# Outer rules\n", encoding="utf-8")
9494
result = find_agents_md_files(str(sub))
95-
self.assertEqual([Path(p) for p in result], [repo / "AGENTS.md"])
95+
self.assertEqual([Path(p) for p in result], [(repo / "AGENTS.md").resolve()])
9696

9797
def test_non_git_dir_is_its_own_bound(self):
9898
"""Outside a repo the search is bounded by the project dir."""
@@ -147,7 +147,7 @@ def test_rendered_as_context_block(self):
147147
result = load_context_files(None, extra_files=find_agents_md_files(d))
148148
self.assertEqual(
149149
result,
150-
f"Request context:\n\nIn file `{agents}`:\n\n# Project Rules\nUse 4 spaces.",
150+
f"Request context:\n\nIn file `{agents.resolve()}`:\n\n# Project Rules\nUse 4 spaces.",
151151
)
152152

153153
def test_empty_file_yields_no_context(self):

‎tests/test_filesystem.py‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -512,11 +512,11 @@ def test_glob_tree_fallback_lists_files_and_depth(self):
512512
self._mkdir("proj", "sub")
513513
open(os.path.join(d, "sub", "b.py"), "w").close()
514514
out = GlobTool().run({"pattern": "*.py", "path": d}, self.ctx)
515-
self.assertIn(os.path.join(d, "a.py"), out)
516-
self.assertIn(os.path.join(d, "sub", "b.py"), out)
515+
self.assertIn(os.path.realpath(os.path.join(d, "a.py")), out)
516+
self.assertIn(os.path.realpath(os.path.join(d, "sub", "b.py")), out)
517517
out1 = GlobTool().run({"pattern": "*.py", "path": d, "depth": 1}, self.ctx)
518-
self.assertIn(os.path.join(d, "a.py"), out1)
519-
self.assertNotIn(os.path.join(d, "sub", "b.py"), out1)
518+
self.assertIn(os.path.realpath(os.path.join(d, "a.py")), out1)
519+
self.assertNotIn(os.path.realpath(os.path.join(d, "sub", "b.py")), out1)
520520

521521
@unittest.skipUnless(shutil.which("git"), "git not available")
522522
def test_glob_and_grep_use_git_backend_in_repo(self):
@@ -527,8 +527,8 @@ def test_glob_and_grep_use_git_backend_in_repo(self):
527527
f.write(content)
528528
subprocess.run(["git", "add", "."], cwd=repo, check=True)
529529
out = GlobTool().run({"pattern": "*", "path": repo}, self.ctx)
530-
self.assertIn(os.path.join(repo, "a.py"), out)
531-
self.assertIn(os.path.join(repo, "b.txt"), out)
530+
self.assertIn(os.path.realpath(os.path.join(repo, "a.py")), out)
531+
self.assertIn(os.path.realpath(os.path.join(repo, "b.txt")), out)
532532
out = Grep().run({"regex": "hello", "path": repo}, self.ctx)
533533
self.assertIn("a.py", out)
534534
self.assertIn("hello", out)
@@ -537,6 +537,27 @@ def test_glob_and_grep_use_git_backend_in_repo(self):
537537
self.assertIn("a.py", out)
538538
self.assertIn("hello", out)
539539

540+
@unittest.skipUnless(shutil.which("git"), "git not available")
541+
def test_glob_and_grep_work_through_symlinked_path(self):
542+
"""Regression: a search path that goes through a symlink (macOS
543+
/var -> /private/var) must be resolved before the git backend
544+
computes relpaths, or git rejects the pathspec as outside the
545+
repository."""
546+
real = self._mkdir("real")
547+
link = os.path.join(self.tmp.name, "link")
548+
os.symlink(real, link)
549+
subprocess.run(["git", "init", "-q", real], check=True)
550+
with open(os.path.join(real, "a.py"), "w") as f:
551+
f.write("hello world\n")
552+
subprocess.run(["git", "add", "."], cwd=real, check=True)
553+
out = GlobTool().run({"pattern": "*", "path": link}, self.ctx)
554+
self.assertNotIn("outside repository", out)
555+
self.assertNotIn("Glob failed", out)
556+
self.assertIn(os.path.realpath(os.path.join(real, "a.py")), out)
557+
out = Grep().run({"regex": "hello", "path": link}, self.ctx)
558+
self.assertIn("a.py", out)
559+
self.assertIn("hello", out)
560+
540561
def test_glob_nonexistent_path_errors(self):
541562
out = GlobTool().run(
542563
{"pattern": "*", "path": os.path.join(self.tmp.name, "nope")}, self.ctx

‎tests/test_invariants.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,32 @@ def test_plan_file_itself_stays_writable_in_plan_mode(self):
780780
with open(plan_file) as f:
781781
self.assertEqual(f.read(), "via link")
782782

783+
def test_plan_file_writable_when_temp_dir_is_symlinked(self):
784+
"""Regression: when the plan dir goes through a symlink (macOS
785+
TMPDIR /var -> /private/var), the plan file must still be
786+
recognized as the plan file and stay writable in plan mode."""
787+
with tempfile.TemporaryDirectory(prefix="pah-prop-plan-") as tmpdir:
788+
real = os.path.join(tmpdir, "real")
789+
os.makedirs(real)
790+
link = os.path.join(tmpdir, "link")
791+
os.symlink(real, link)
792+
with mock.patch(
793+
"python_agent_harness.planmode._plan_temp_dir",
794+
return_value=link,
795+
):
796+
session = self.make_plan_session(tmpdir)
797+
plan_file = session.plan_mode.plan_file
798+
self.assertTrue(os.path.isfile(plan_file))
799+
dirname, basename = os.path.split(plan_file)
800+
r = session.execute_tool(
801+
"Write",
802+
{"path": dirname, "filename": basename, "content": "# plan\n"},
803+
call_id="w1",
804+
)
805+
self.assertNotIn("blocked by plan mode", r)
806+
with open(plan_file) as f:
807+
self.assertEqual(f.read(), "# plan\n")
808+
783809
def test_read_only_tools_pass_through_plan_mode(self):
784810
with tempfile.TemporaryDirectory(prefix="pah-prop-plan-") as tmpdir:
785811
session = self.make_plan_session(tmpdir)

‎tests/test_prompts.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ def test_index_keyed_by_frontmatter_name(self):
200200
encoding="utf-8",
201201
)
202202
index = index_skills(d)
203-
self.assertEqual(index, {"天气预报助手": (str(skill), "does things")})
203+
self.assertEqual(index, {"天气预报助手": (os.path.realpath(skill), "does things")})
204204

205205
def test_no_frontmatter_name_skipped(self):
206206
with tempfile.TemporaryDirectory() as d:
@@ -219,7 +219,7 @@ def test_symlinked_skill_dir_indexed(self):
219219
skill = sub / "SKILL.md"
220220
skill.write_text("---\nname: linked-skill\n---\nbody", encoding="utf-8")
221221
os.symlink(sub, Path(d) / "linked")
222-
self.assertEqual(index_skills(d)["linked-skill"][0], str(skill))
222+
self.assertEqual(index_skills(d)["linked-skill"][0], os.path.realpath(skill))
223223

224224

225225
class TestLoadContextFiles(unittest.TestCase):

‎tests/test_session.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,10 @@ def test_skill_found_by_frontmatter_name(self):
152152
with open(os.path.join(sub, "SKILL.md"), "w") as f:
153153
f.write("---\nname: my-rules\ndescription: rules\n---\n# Rules")
154154
session._skill_dir = d
155-
self.assertEqual(session.find_skill("my-rules"), os.path.join(sub, "SKILL.md"))
155+
self.assertEqual(
156+
session.find_skill("my-rules"),
157+
os.path.realpath(os.path.join(sub, "SKILL.md")),
158+
)
156159

157160
def test_frontmatter_name_differs_from_directory_name(self):
158161
"""Regression test: the advertised name (frontmatter) must
@@ -165,7 +168,10 @@ def test_frontmatter_name_differs_from_directory_name(self):
165168
with open(os.path.join(sub, "SKILL.md"), "w") as f:
166169
f.write("---\nname: 天气预报助手\n---\n# body")
167170
session._skill_dir = d
168-
self.assertEqual(session.find_skill("天气预报助手"), os.path.join(sub, "SKILL.md"))
171+
self.assertEqual(
172+
session.find_skill("天气预报助手"),
173+
os.path.realpath(os.path.join(sub, "SKILL.md")),
174+
)
169175

170176
def test_nested_skill_dir_discovered(self):
171177
session = RecordingSession()
@@ -175,7 +181,10 @@ def test_nested_skill_dir_discovered(self):
175181
with open(os.path.join(nested, "SKILL.md"), "w") as f:
176182
f.write("---\nname: deep-skill\n---\nbody")
177183
session._skill_dir = d
178-
self.assertEqual(session.find_skill("deep-skill"), os.path.join(nested, "SKILL.md"))
184+
self.assertEqual(
185+
session.find_skill("deep-skill"),
186+
os.path.realpath(os.path.join(nested, "SKILL.md")),
187+
)
179188

180189
def test_flat_file_without_frontmatter_returns_none(self):
181190
"""Flat files (skills/style.md) are no longer resolvable: only
@@ -210,7 +219,10 @@ def test_symlinked_skill_dir_resolves(self):
210219
f.write("---\nname: linked-skill\n---\n# Linked")
211220
os.symlink(sub, os.path.join(d, "linked"))
212221
session._skill_dir = d
213-
self.assertEqual(session.find_skill("linked-skill"), os.path.join(sub, "SKILL.md"))
222+
self.assertEqual(
223+
session.find_skill("linked-skill"),
224+
os.path.realpath(os.path.join(sub, "SKILL.md")),
225+
)
214226

215227
def test_traversal_skill_returns_none(self):
216228
"""Path-traversal inputs are inert: lookup only matches names in

0 commit comments

Comments
 (0)