Skip to content

Commit c0635da

Browse files
committed
install-script fix
1 parent 992b3ef commit c0635da

5 files changed

Lines changed: 117 additions & 15 deletions

File tree

System/scripts/install.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ def _parse_argv(argv: list[str]) -> argparse.Namespace:
6161
p_update.add_argument("--target", type=Path, default=Path.cwd())
6262
p_update.add_argument("--prune", action="store_true",
6363
help="Remove symlinks whose source no longer exists")
64+
p_update.add_argument("--no-gitignore", action="store_true",
65+
help="Re-sync symlinks without touching .gitignore")
6466

6567
p_uninstall = sub.add_parser("uninstall", help="Remove framework artifacts")
6668
p_uninstall.add_argument("--target", type=Path, default=Path.cwd())

System/scripts/installer/cli.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -505,18 +505,23 @@ def _cmd_update(args) -> int:
505505
if args.prune:
506506
pruned += _prune_stale_symlinks(target_dir, agentic)
507507

508-
# Refresh the .gitignore block. Two reasons this cannot be skipped: linking
509-
# and pruning above change which entries are project-local, so the
510-
# `!`-exception list goes stale; and a framework upgrade may change the block
511-
# body itself, which is the only way a shipped rule fix reaches an existing
512-
# project. `switch` gets this via _cmd_install — `update` used to miss it.
513-
try:
514-
state["gitignore_block_hash"] = update_gitignore(target, profile, state)
515-
except IntegrityError as exc:
516-
# The block was hand-edited. The symlink re-sync above already succeeded,
517-
# so warn and keep the run green rather than aborting half-done; `install
518-
# --force` is the deliberate way to overwrite a customised block.
519-
print(f"warning: .gitignore block left untouched — {exc}", flush=True)
508+
# Refresh the .gitignore block. Two reasons it is on by default: linking and
509+
# pruning above change which entries are project-local, so the `!`-exception
510+
# list goes stale; and a framework upgrade may change the block body itself,
511+
# which is the only way a shipped rule fix reaches an existing project.
512+
# `switch` gets this via _cmd_install — `update` used to miss it. `install`
513+
# has always offered an opt-out; `update` now offers the same one, so a
514+
# project that manages its own .gitignore is not forced to hand-edit the
515+
# block (which costs it every future block fix).
516+
if not getattr(args, "no_gitignore", False):
517+
try:
518+
state["gitignore_block_hash"] = update_gitignore(target, profile, state)
519+
except IntegrityError as exc:
520+
# The block was hand-edited. The symlink re-sync above already
521+
# succeeded, so warn and keep the run green rather than aborting
522+
# half-done; `install --force` is the deliberate way to overwrite a
523+
# customised block.
524+
print(f"warning: .gitignore block left untouched — {exc}", flush=True)
520525

521526
state["managed_paths"] = state_mod.collect_managed_symlinks(target)
522527
state_mod.save_state(target, state)

System/scripts/installer/gitignore.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,30 @@ def _points_into_agentic(entry: Path, target: Path) -> bool:
2828
return resolves_inside(entry, Path(target) / ".agentic-development")
2929

3030

31+
def _survives_a_clone(entry: Path, target: Path) -> bool:
32+
"""True if a fresh clone of ``target`` would still resolve ``entry``.
33+
34+
A real file or directory always qualifies. A symlink qualifies only when it
35+
resolves back inside the project, because its destination is then part of
36+
the same repository. A symlink leaving the project — ``.claude/skills/x`` →
37+
``~/ExternalTools/...`` — names a path that exists on this machine alone, so
38+
committing it hands every other clone a dangling link.
39+
40+
This is the same argument :func:`build_block_body` already makes about the
41+
framework's own symlink, which is why that pattern carries no trailing slash.
42+
"""
43+
if not entry.is_symlink():
44+
return True
45+
return resolves_inside(entry, target)
46+
47+
3148
def scan_local_exceptions(target: Path) -> list[str]:
3249
"""Return sorted ``!/path`` lines for project-local (non-framework) entries.
3350
3451
An entry is project-local when it is not a symlink, or is a symlink that
35-
does not resolve into ``.agentic-development/``. Dotfiles are skipped.
36-
A broken framework symlink is reported as a warning and not emitted.
52+
resolves inside the project and not into ``.agentic-development/``. Dotfiles
53+
are skipped. Two kinds of entry are reported as a warning and not emitted: a
54+
broken framework symlink, and a symlink pointing outside the project.
3755
"""
3856
target = Path(target)
3957
exceptions: list[str] = []
@@ -52,6 +70,14 @@ def scan_local_exceptions(target: Path) -> list[str]:
5270
flush=True,
5371
)
5472
continue # framework symlink — never a project-local exception
73+
if not _survives_a_clone(entry, target):
74+
print(
75+
f"warning: {entry} resolves outside the project "
76+
f"({Path(entry).resolve()}) — left ignored, because a "
77+
f"committed link to it would dangle on every other clone",
78+
flush=True,
79+
)
80+
continue
5581
exceptions.append(f"!/{rel}/{entry.name}")
5682
return sorted(exceptions)
5783

tests/installer/test_gitignore.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
"""
55
from __future__ import annotations
66

7+
import io
78
import os
9+
from contextlib import redirect_stdout
10+
from pathlib import Path
811

912
from _base import FRAMEWORK_ROOT, InstallerTestCase
1013
from installer.errors import IntegrityError
@@ -104,6 +107,55 @@ def test_broken_framework_symlink_warned_not_added(self) -> None:
104107
result = scan_local_exceptions(self.target)
105108
self.assertEqual(result, []) # broken framework link is not an exception
106109

110+
# -- symlinks leaving the project ------------------------------------
111+
# A `!` on one of these invites `git add -A` to commit a path that exists on
112+
# the author's machine only. Measured on a real project: one `update` added
113+
# 15 such lines for skills symlinked out of ~/ExternalTools.
114+
115+
def _outbound_symlink(self, name: str) -> Path:
116+
"""Symlink ``name`` at a real directory outside the target project."""
117+
outside = self.tmp / "outside" / name
118+
outside.mkdir(parents=True)
119+
os.symlink(outside, self.skills / name)
120+
return outside
121+
122+
def test_outbound_symlink_not_exception(self) -> None:
123+
self._outbound_symlink("external-skill")
124+
self.assertEqual(scan_local_exceptions(self.target), [])
125+
126+
def test_outbound_symlink_warned(self) -> None:
127+
self._outbound_symlink("external-skill")
128+
with redirect_stdout(io.StringIO()) as out:
129+
scan_local_exceptions(self.target)
130+
self.assertIn("external-skill", out.getvalue())
131+
self.assertIn("resolves outside the project", out.getvalue())
132+
133+
def test_inbound_symlink_is_exception(self) -> None:
134+
# `.claude/skills/x` -> `.agent/skills/x` is relative and in-repo, so a
135+
# clone resolves it; it stays trackable.
136+
(self.skills / "owned").mkdir()
137+
claude_skills = self.target / ".claude" / "skills"
138+
claude_skills.mkdir(parents=True)
139+
os.symlink("../../.agent/skills/owned", claude_skills / "owned")
140+
self.assertEqual(
141+
scan_local_exceptions(self.target),
142+
["!/.agent/skills/owned", "!/.claude/skills/owned"],
143+
)
144+
145+
def test_outbound_symlink_stays_out_across_repeated_updates(self) -> None:
146+
# The block is regenerated on every `update`, so a one-off correct run
147+
# proves nothing: the defect must not come back on the second pass.
148+
self._outbound_symlink("external-skill")
149+
state: dict = {}
150+
with redirect_stdout(io.StringIO()):
151+
state["gitignore_block_hash"] = update_gitignore(
152+
self.target, _CLAUDE, state)
153+
update_gitignore(self.target, _CLAUDE, state)
154+
body = extract_block(
155+
(self.target / ".gitignore").read_text(encoding="utf-8"),
156+
GITIGNORE_MARKERS)
157+
self.assertNotIn("external-skill", body)
158+
107159

108160
class TestUpdateGitignore(InstallerTestCase):
109161

tests/installer/test_subcommands.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def _uninstall_ns(target, **kw) -> argparse.Namespace:
3939

4040

4141
def _update_ns(target, **kw) -> argparse.Namespace:
42-
d = dict(command="update", target=target, prune=False)
42+
d = dict(command="update", target=target, prune=False, no_gitignore=False)
4343
d.update(kw)
4444
return argparse.Namespace(**d)
4545

@@ -190,6 +190,23 @@ def test_update_refreshes_stale_gitignore_block(self) -> None:
190190
self.assertIn("/.agentic-development", lines)
191191
self.assertNotIn("/stale-rule", lines)
192192

193+
def test_update_no_gitignore_leaves_the_file_alone(self) -> None:
194+
# `install` has always offered this opt-out; `update` did not, so a
195+
# project that manages its own .gitignore had to hand-edit the managed
196+
# block — which costs it every future shipped block fix.
197+
target = self.make_target()
198+
main(_install_ns(target))
199+
gi = target / ".gitignore"
200+
self._restate_gitignore(target, "/.agentic-development/\n/stale-rule")
201+
before = gi.read_text()
202+
state_file = target / ".agentic-installer-state.json"
203+
hash_before = json.loads(state_file.read_text())["gitignore_block_hash"]
204+
self.assertEqual(main(_update_ns(target, no_gitignore=True)), 0)
205+
self.assertEqual(gi.read_text(), before)
206+
self.assertEqual(
207+
json.loads(state_file.read_text())["gitignore_block_hash"],
208+
hash_before)
209+
193210
def test_update_survives_hand_edited_gitignore(self) -> None:
194211
# Block edited by hand => hash mismatch. The symlink re-sync has already
195212
# succeeded, so update warns and stays green instead of aborting midway,

0 commit comments

Comments
 (0)