fix: avoid external_directory prompt for global AGENTS.md

[Haohao-end]

Issue for this PR

Closes #13751

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

This fixes a case where ~/.config/opencode/AGENTS.md (and OPENCODE_CONFIG_DIR/AGENTS.md) could be treated as an external path and trigger an external_directory permission prompt.

The root cause was that the instruction/config system already knows about OpenCode's global config directories, but the external-directory permission check only trusted the current project/worktree. Because of that mismatch, AGENTS files inside OpenCode's own global config locations were incorrectly treated as external.

To fix it, I:

• added a shared helper for OpenCode global config directories in ConfigPaths
• updated assertExternalDirectory() to treat those config directories as internal/trusted paths
• added regression tests for both Global.Path.config/AGENTS.md and OPENCODE_CONFIG_DIR/AGENTS.md

This keeps the permission check strict for normal paths outside the project, while avoiding prompts for OpenCode's own config directories.

How did you verify your code works?

I verified it locally with:

• bun test test/tool/external-directory.test.ts
• bun typecheck

I also added regression coverage to make sure:

• Global.Path.config/AGENTS.md does not trigger external_directory
• OPENCODE_CONFIG_DIR/AGENTS.md does not trigger external_directory
• regular external directories still go through the permission flow

Screenshots / recordings

N/A (non-UI change)

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

Found a potential duplicate:

Related PR:

• fix(read): skip external directory prompt for global AGENTS.md #13753 - fix(read): skip external directory prompt for global AGENTS.md
  fix(read): skip external directory prompt for global AGENTS.md #13753

Why it's related: This PR appears to address the exact same issue - skipping the external directory prompt for global AGENTS.md files. Both PRs are fixing issue #13751 and target the same problem of preventing unnecessary external_directory permission prompts for global configuration files. You should verify if #13753 was already merged or if it needs to be updated rather than creating a separate PR.

[atharvau]

Code Review - PR #18721

✅ Overall Assessment

This PR successfully addresses the external directory prompt issue for global AGENTS.md files. The implementation is clean and well-tested.

🔍 Code Quality Analysis

Bugs: None found

• Logic is sound and handles both absolute and relative paths correctly
• Proper path resolution using path.resolve and path.join

Security: ✅ Good

• No security vulnerabilities identified
• Path handling is secure with proper validation

Performance: ✅ Good

• Efficient implementation with minimal overhead
• globalDirectories() function avoids repeated array construction

Style: ✅ Excellent

• Follows project naming conventions (single-word function name internal)
• Clean separation of concerns
• Good test coverage with comprehensive scenarios

🧪 Test Coverage

• ✅ All tests pass (5/5)
• New tests cover both Global.Path.config and OPENCODE_CONFIG_DIR scenarios
• Edge cases properly handled

📝 Suggestions

1. Minor: Consider adding a JSDoc comment to the internal function to explain its purpose
2. Consider: The globalDirectories() function could be memoized if called frequently, though current usage seems fine

✅ Approval Recommendation

This PR is ready to merge. It's a solid fix with good test coverage and no identified issues.

[Haohao-end]

Thanks for the review!

One thing I noticed is that this may overlap with #13753, which appears to be addressing the same issue (#13751). I’m happy to close this PR if maintainers would prefer to keep the earlier one, but if this implementation is preferred I can also make any follow-up changes needed.

[atharvau]

Code Review Summary

Overall Assessment: ✅ LGTM - Good security improvement

Security: ✅ Important improvement

• Enhancement: Prevents unnecessary external directory prompts for global config files
• Logic: Properly identifies files within global config directories as internal
• Security boundary: Maintains proper separation between project and global scope

Bugs: ✅ Correct fix

• Fixes unnecessary permission prompts when accessing global AGENTS.md files
• Proper handling of both Global.Path.config and OPENCODE_CONFIG_DIR
• Logic correctly handles both absolute and relative paths

Performance: ✅ Minimal impact

• Added path resolution logic is efficient
• Early return pattern maintains good performance
• No unnecessary file system operations

Style: ✅ Clean implementation

• Good function naming: internal() is clear and descriptive
• Proper use of existing utilities (Instance.containsPath, Filesystem.contains)
• Clean separation of concerns with ConfigPaths.globalDirectories()

Test Coverage: ✅ Excellent

• Comprehensive test coverage for both config directory scenarios
• Tests cover Global.Path.config and OPENCODE_CONFIG_DIR cases
• Good edge case handling in tests with proper setup/teardown

Breaking Changes: ✅ None

• Backward compatible change that only improves user experience
• No API changes

Architecture Note:
The refactoring of ConfigPaths.globalDirectories() improves code reuse and maintains consistency between configuration path resolution and external directory checking.

Minor suggestion: Consider adding a JSDoc comment to the internal() helper function to document its purpose for future maintainers.

[atharvau]

Code Review Comments

✅ Excellent Bug Fix

This addresses a legitimate UX friction where OpenCode's own config files were incorrectly treated as external.

🔍 Analysis

Problem: Global AGENTS.md files (in ~/.config/opencode/ or OPENCODE_CONFIG_DIR) triggered unnecessary external_directory permission prompts.

Root Cause: Mismatch between config system (knows global dirs) and permission system (only trusts project paths).

Solution: Centralize global directory logic and extend permission system to trust OpenCode's config locations.

✅ What's Great

1. Root Cause Analysis: Correctly identified the system boundary mismatch

2. Clean Architecture:

   • Extracted globalDirectories helper for reuse
   • Updated both config and permission systems consistently

3. Comprehensive Testing:

   • Tests both Global.Path.config and OPENCODE_CONFIG_DIR scenarios
   • Maintains existing external directory protection
   • Proper environment variable cleanup in tests

4. Security Conscious: Only trusts OpenCode's own config dirs, not arbitrary paths

🔵 Implementation Details

1. Path Resolution: Uses path.resolve and path.join correctly for cross-platform compatibility

2. Relative Path Handling: Correctly resolves relative paths against Instance.directory

3. Environment Variable Handling: Properly handles undefined OPENCODE_CONFIG_DIR

💡 Code Quality

1. Function Naming: internal clearly indicates purpose
2. Logic Flow: Early returns make the code readable
3. Test Coverage: Good edge case coverage with proper setup/teardown

🔴 Minor Observations

1. Performance: globalDirectories is called on every check - consider caching if this becomes a hot path

2. Path Normalization: Could consider using path.normalize for extra safety, though current implementation looks solid

🎯 Verdict

APPROVE - This is a well-implemented fix for a real usability issue. The solution is architecturally sound and maintains security boundaries while fixing the UX friction.

Great attention to test coverage and edge cases. The centralized global directory logic is a good architectural improvement.

[atharvau]

Code Review: Fix Global AGENTS.md Permission Issue

Summary

This PR fixes an issue where accessing global AGENTS.md files (in Global.Path.config or OPENCODE_CONFIG_DIR) would incorrectly prompt for external directory permissions. The fix treats these global configuration paths as "internal" to avoid unnecessary permission prompts.

✅ Positives

• Clear problem identification: Global configuration files should not require external directory permissions
• Well-structured solution: Created globalDirectories() helper and internal() function for clean separation of concerns
• Comprehensive test coverage: Added tests for both Global.Path.config and OPENCODE_CONFIG_DIR scenarios
• Proper path resolution: Uses path.resolve() and Filesystem.contains() for accurate path matching
• No breaking changes: Maintains existing behavior for actual external directories

🔍 Analysis

Root Cause

The original Instance.containsPath(target) check only verified if a file was within the current project instance, but didn't account for global configuration directories that should also be considered "internal".

Technical Implementation

1. ConfigPaths.globalDirectories(): Centralizes logic for determining global config paths
2. internal() function: Combines project instance check with global directory checks
3. Path handling: Properly handles both absolute and relative paths
4. Filesystem.contains(): Uses existing utility for safe path containment checks

Security Analysis

• ✅ Maintains security boundaries: Only allows access to legitimate global config dirs
• ✅ No privilege escalation: Doesn't bypass security for arbitrary external paths
• ✅ Proper path validation: Uses established path resolution utilities

✅ Code Quality

• Clean refactoring: globalDirectories() eliminates code duplication between directories() function
• Consistent naming: Follows existing patterns in codebase
• Good test coverage: Tests cover both config directory scenarios with proper setup/teardown
• Proper error handling: Maintains existing error paths for actual external directories

✅ Tests Pass

All tests pass successfully ✓
New test cases specifically validate the fix works correctly.

💭 Minor Considerations

Edge Cases

Consider whether symbolic links pointing to global config directories should also be treated as internal (current implementation may not handle this case).

Documentation

The behavior change might be worth documenting in the AGENTS.md configuration documentation to clarify that global AGENTS.md files don't require external directory permissions.

✅ Performance Impact

• Minimal performance impact (adds one additional path containment check)
• More efficient than showing permission prompts for global files
• Good use of existing utilities

Verdict: LGTM ✅
This is a targeted fix that resolves a legitimate UX issue without compromising security. The implementation is clean, well-tested, and maintains proper separation of concerns.

[atharvau]

Smart Configuration Path Fix

This PR improves the external directory check to avoid unnecessary prompts for global configuration files like AGENTS.md.

Key improvements:

• New globalDirectories helper for clean abstraction
• Enhanced internal path detection covering both Global.Path.config and OPENCODE_CONFIG_DIR
• Comprehensive test coverage for both config directory scenarios
• Proper path resolution and containment checking

The solution correctly identifies when files are in legitimate global config locations and bypasses the external directory prompt. The test coverage ensures this works for both standard and custom config directories.

This improves user experience by reducing friction for legitimate config file access.

Ready to merge.

[atharvau]

Code Review Summary

Overall Assessment: APPROVED - This PR properly fixes external directory prompts for global AGENTS.md files.

Strengths:

1. Security improvement: Prevents unnecessary permission prompts for trusted global config directories.
2. Comprehensive solution: Handles both Global.Path.config and OPENCODE_CONFIG_DIR scenarios.
3. Proper path resolution: Uses StoredPath.parse and Filesystem.contains for robust path checking.
4. Excellent test coverage: Tests cover both global config scenarios with proper cleanup.

Technical Implementation:

• The internal function correctly identifies files within project or global config directories
• Uses path.isAbsolute and path.join for safe path operations
• Proper handling of relative vs absolute paths
• Good separation of concerns with dedicated helper function

Code Quality:

• Clean, readable code structure
• Comprehensive test coverage with edge cases
• Proper error handling and cleanup in tests

No issues found. This is a well-implemented security and UX improvement.

[atharvau]

Code Review - PR #18721: Avoid external_directory prompt for global AGENTS.md

✅ LOOKS GOOD - Important security improvement

Review Summary

This PR fixes external directory prompts when accessing AGENTS.md files from global config directories, improving UX for legitimate global configuration access.

✅ Strengths

• Security Enhancement: Prevents unnecessary prompts for trusted global config directories
• Proper Scope: Covers both Global.Path.config and OPENCODE_CONFIG_DIR
• Test Coverage: Excellent comprehensive tests covering both scenarios
• Path Safety: Uses proper path resolution and containment checks

Code Quality

• ✅ internal() helper function properly validates trusted paths
• ✅ Uses StoredPath.parse() for consistent path handling
• ✅ Leverages existing Filesystem.contains() for security checks
• ✅ Tests verify no prompts are triggered for legitimate global configs

Security Considerations

• ✅ Still maintains security for truly external files
• ✅ Only exempts files within known global config directories
• ✅ Preserves existing project containment logic

Recommendation: Ready to merge! This improves UX while maintaining security.

[atharvau]

Code Review Summary

This PR fixes an important UX issue where users were unnecessarily prompted for permissions when accessing global AGENTS.md files. The solution is well-designed and includes proper test coverage.

✅ Problem Solved

• UX Improvement: Eliminates unnecessary permission prompts for global config files
• Proper Scoping: Global configuration files should not trigger external directory prompts
• Maintains Security: Only bypasses prompts for legitimate global config locations

✅ Implementation Quality

1. Clean Abstraction

function internal(target: string) {
  const file = path.isAbsolute(target) ? target : path.join(Instance.directory, target)
  if (Instance.containsPath(file)) return true
  return ConfigPaths.globalDirectories().some((dir) => Filesystem.contains(path.resolve(dir), file))
}

Good separation of concerns - centralizes the "internal file" logic.

2. Reusable Helper
ConfigPaths.globalDirectories() provides a single source of truth for global config locations, used in both config path resolution and permission checking.

3. Comprehensive Tests
Tests cover both Global.Path.config and OPENCODE_CONFIG_DIR scenarios with proper environment variable handling.

✅ Security Considerations

• Maintains Security Boundaries: Only bypasses prompts for explicitly configured global directories
• Path Resolution: Properly handles both absolute and relative paths
• No Privilege Escalation: Doesn't introduce new security risks

⚠️ Minor Observations

1. Function Naming
The internal() function could be more descriptive. Consider isInternalPath() or isAllowedPath() for clarity.

2. Path Resolution
The logic handles relative paths by joining with Instance.directory, which seems appropriate but might warrant a comment explaining this behavior.

🔧 Optional Improvements

1. Add JSDoc to internal() function explaining the path resolution logic
2. Consider more descriptive name for the internal helper function

📊 Assessment

• UX: ✅ Excellent - eliminates unnecessary user friction
• Security: ✅ Safe - maintains appropriate boundaries
• Code Quality: ✅ Good - clean, tested implementation
• Breaking Changes: ✅ None - pure improvement

Recommendation: Approve - this is a thoughtful fix that improves user experience while maintaining security.

[atharvau]

Code Review: fix: avoid external_directory prompt for global AGENTS.md

✅ APPROVED WITH SUGGESTIONS

This is a solid fix addressing an annoying UX issue where users get permission prompts for accessing global configuration files.

Problem Identified: assertExternalDirectory was prompting users when accessing AGENTS.md files in global config directories, even though these are legitimate configuration files.

Solution Analysis:

• ✅ Added globalDirectories() helper for DRY config path management
• ✅ Enhanced internal() function to check both project and global config paths
• ✅ Comprehensive test coverage for both Global.Path.config and OPENCODE_CONFIG_DIR
• ✅ Proper path resolution with path.resolve() and Filesystem.contains()

Technical Quality:

• Good separation of concerns with the new helper function
• Proper absolute path handling for relative targets
• Maintains existing security by only exempting known global directories
• Clean test structure with proper environment variable cleanup

🔶 MINOR SUGGESTIONS

1. Consider Adding JSDoc

2. Edge Case Handling

The path.isAbsolute(target) check is good, but consider documenting the behavior when Instance.directory might be undefined.

3. Performance Note

The function now calls ConfigPaths.globalDirectories() on every check. If this becomes a hot path, consider caching the result.

Security Review: ✅ Only exempts paths within known global config directories, maintains security boundaries.

This fixes a legitimate UX annoyance without compromising security. Ready to merge! 📁

[atharvau]

Code Review: fix: avoid external_directory prompt for global AGENTS.md

✅ APPROVED WITH SUGGESTIONS

This is a solid fix addressing an annoying UX issue where users get permission prompts for accessing global configuration files.

Problem Identified: assertExternalDirectory was prompting users when accessing AGENTS.md files in global config directories, even though these are legitimate configuration files.

Solution Analysis:

• ✅ Added globalDirectories() helper for DRY config path management
• ✅ Enhanced internal() function to check both project and global config paths
• ✅ Comprehensive test coverage for both Global.Path.config and OPENCODE_CONFIG_DIR
• ✅ Proper path resolution with path.resolve() and Filesystem.contains()

Technical Quality:

• Good separation of concerns with the new helper function
• Proper absolute path handling for relative targets
• Maintains existing security by only exempting known global directories
• Clean test structure with proper environment variable cleanup

🔶 MINOR SUGGESTIONS

1. Performance Note

The function now calls ConfigPaths.globalDirectories() on every check. If this becomes a hot path, consider caching the result.

2. Edge Case Handling

The path.isAbsolute(target) check is good, but consider documenting behavior when Instance.directory might be undefined.

Security Review: ✅ Only exempts paths within known global config directories, maintains security boundaries.

This fixes a legitimate UX annoyance without compromising security. Ready to merge! 📁

[atharvau]

Code Review: Fix external_directory prompt for global AGENTS.md

✅ APPROVED - Good fix for configuration file access

Summary

This PR fixes an issue where accessing AGENTS.md files in global OpenCode config directories would incorrectly trigger external_directory permission prompts.

Analysis

Problem Identified ✅

• AGENTS.md files in global config directories were treated as external
• This caused unnecessary permission prompts for legitimate configuration files
• Users would be asked to approve access to their own global config files

Solution Quality ✅

1. New globalDirectories() helper: Centralizes logic for identifying global config paths
2. Enhanced internal() function: Now checks if a file is within project OR global config directories
3. Improved path resolution: Uses path.resolve() and Filesystem.contains() for proper containment checking
4. Comprehensive test coverage: Added tests for both global config scenarios

Code Quality Analysis

Strengths ✅

• Clean abstraction: globalDirectories() provides reusable logic
• Proper path handling: Uses absolute paths with path.resolve()
• Consistent with existing patterns: Follows established codebase conventions
• Good test coverage: Tests both global config scenarios thoroughly
• No breaking changes: Maintains existing API contracts

Security & Performance

• Security: ✅ Improves UX without reducing security - still blocks truly external files
• Performance: ✅ Minimal overhead from additional containment checks
• Correctness: ✅ Fixes legitimate usability issue

Recommendation: MERGE - This is a well-implemented fix for a legitimate UX issue with comprehensive test coverage.