-
Notifications
You must be signed in to change notification settings - Fork 53k
fix(context): dedupe subdirectory hints by content digest and skip backup/vendor dirs #72448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| Inspired by Block/goose's SubdirectoryHintTracker. | ||
| """ | ||
|
|
||
| import hashlib | ||
| import logging | ||
| import os | ||
| import shlex | ||
|
|
@@ -45,6 +46,18 @@ | |
| # Prevents scanning all the way to / for deeply nested paths. | ||
| _MAX_ANCESTOR_WALK = 5 | ||
|
|
||
| # Directory names that never contain authoritative project context. | ||
| # Backups, vendored deps, VCS internals, and caches routinely hold *copies* of | ||
| # AGENTS.md; loading those duplicates real context and inflates the prompt. | ||
| _EXCLUDED_DIR_NAMES = frozenset({ | ||
| "node_modules", "venv", ".venv", "__pycache__", | ||
| ".git", ".hg", ".svn", | ||
| ".Trash", ".cache", ".tox", ".mypy_cache", ".pytest_cache", | ||
| "site-packages", "dist-packages", | ||
| "backups", "backup", ".backups", | ||
| "vendor", "third_party", | ||
| }) | ||
|
|
||
| class SubdirectoryHintTracker: | ||
| """Track which directories the agent visits and load hints on first access. | ||
|
|
||
|
|
@@ -61,8 +74,34 @@ class SubdirectoryHintTracker: | |
| def __init__(self, working_dir: Optional[str] = None): | ||
| self.working_dir = Path(working_dir or os.getcwd()).resolve() | ||
| self._loaded_dirs: Set[Path] = set() | ||
| # Content digests already injected — prevents re-sending the same file | ||
| # reachable through symlinks, hardlinks, or duplicated copies. | ||
| self._loaded_digests: Set[str] = set() | ||
| # Pre-mark the working dir as loaded (startup context handles it) | ||
| self._loaded_dirs.add(self.working_dir) | ||
| self._seed_working_dir_digest() | ||
|
|
||
| def _seed_working_dir_digest(self) -> None: | ||
| """Record the CWD context file's digest so it is never re-injected. | ||
|
|
||
| ``prompt_builder`` already loads the working directory's context file at | ||
| startup. Seeding its digest here means the same content reached through | ||
| a different path (a symlink farm, a shared workspace) is recognised as a | ||
| duplicate instead of being sent a second time. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seed is unconditional, but |
||
| """ | ||
| for filename in _HINT_FILENAMES: | ||
| candidate = self.working_dir / filename | ||
| try: | ||
| if not candidate.is_file(): | ||
| continue | ||
| content = candidate.read_text(encoding="utf-8").strip() | ||
| except (OSError, UnicodeDecodeError): | ||
| continue | ||
| if content: | ||
| self._loaded_digests.add( | ||
| hashlib.sha256(content.encode("utf-8")).hexdigest() | ||
| ) | ||
| break # first match wins, mirroring startup loading | ||
|
|
||
| def check_tool_call( | ||
| self, | ||
|
|
@@ -166,8 +205,24 @@ def _is_valid_subdir(self, path: Path) -> bool: | |
| return False | ||
| if path in self._loaded_dirs: | ||
| return False | ||
| if self._is_excluded(path): | ||
| return False | ||
| return True | ||
|
|
||
| def _is_excluded(self, path: Path) -> bool: | ||
| """True when the path sits inside a directory that holds copies, not context. | ||
|
|
||
| Directories the user is deliberately working inside are never excluded — | ||
| if ``working_dir`` is itself under ``vendor/``, that segment is legitimate | ||
| and only segments *below* the working dir are screened. | ||
| """ | ||
| try: | ||
| rel_parts = path.relative_to(self.working_dir).parts | ||
| except ValueError: | ||
| # Outside the working dir — screen every segment. | ||
| rel_parts = path.parts | ||
| return any(part in _EXCLUDED_DIR_NAMES for part in rel_parts) | ||
|
|
||
| def _load_hints_for_directory(self, directory: Path) -> Optional[str]: | ||
| """Load hint files from a directory. Returns formatted text or None.""" | ||
| self._loaded_dirs.add(directory) | ||
|
|
@@ -184,6 +239,19 @@ def _load_hints_for_directory(self, directory: Path) -> Optional[str]: | |
| content = hint_path.read_text(encoding="utf-8").strip() | ||
| if not content: | ||
| continue | ||
| # Skip content we've already injected. The same AGENTS.md is | ||
| # routinely reachable through several paths (symlinked shared | ||
| # workspaces, hardlinks, copied backups); re-sending it burns | ||
| # context for zero new information. | ||
| digest = hashlib.sha256(content.encode("utf-8")).hexdigest() | ||
| if digest in self._loaded_digests: | ||
| logger.debug( | ||
| "Skipping duplicate hint content at %s (digest %s)", | ||
| hint_path, | ||
| digest[:12], | ||
| ) | ||
| break | ||
| self._loaded_digests.add(digest) | ||
| # Same security scan as startup context loading | ||
| content = _scan_context_content(content, filename) | ||
| if len(content) > _MAX_HINT_CHARS: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vendoris not reliably a copy-only directory: this suppressesproject/vendor/component/AGENTS.mdeven when the agent is working in that owned nested component. The current exemption only covers a working directory already insidevendor; please preserve or explicitly scope discovery for nested projects.