Skip to content

Commit 34747c0

Browse files
Copilotalexec
andauthored
Skip download/cleanup for local search paths (#171)
* Initial plan * Add local path detection to avoid downloading/deleting local directories Co-authored-by: alexec <1142830+alexec@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
1 parent 51b6a24 commit 34747c0

3 files changed

Lines changed: 300 additions & 0 deletions

File tree

integration_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,3 +1491,144 @@ This is the task prompt for resume mode.
14911491
t.Errorf("stderr should NOT contain 'Rules written' message in resume mode")
14921492
}
14931493
}
1494+
1495+
// TestLocalDirectoryNotDeleted verifies that local directories passed via -d flag
1496+
// are not deleted after the command completes.
1497+
func TestLocalDirectoryNotDeleted(t *testing.T) {
1498+
// Create a local directory with a rule file and a marker file
1499+
localDir := t.TempDir()
1500+
rulesDir := filepath.Join(localDir, ".agents", "rules")
1501+
1502+
if err := os.MkdirAll(rulesDir, 0o755); err != nil {
1503+
t.Fatalf("failed to create rules dir: %v", err)
1504+
}
1505+
1506+
// Create a rule file
1507+
ruleFile := filepath.Join(rulesDir, "local-rule.md")
1508+
ruleContent := `---
1509+
language: go
1510+
---
1511+
# Local Rule
1512+
1513+
This is a rule from a local directory.
1514+
`
1515+
if err := os.WriteFile(ruleFile, []byte(ruleContent), 0o644); err != nil {
1516+
t.Fatalf("failed to write rule file: %v", err)
1517+
}
1518+
1519+
// Create a marker file to verify the directory is not deleted
1520+
markerFile := filepath.Join(localDir, "marker.txt")
1521+
if err := os.WriteFile(markerFile, []byte("marker"), 0o644); err != nil {
1522+
t.Fatalf("failed to write marker file: %v", err)
1523+
}
1524+
1525+
// Create a temporary directory for the task
1526+
tmpDir := t.TempDir()
1527+
tasksDir := filepath.Join(tmpDir, ".agents", "tasks")
1528+
1529+
if err := os.MkdirAll(tasksDir, 0o755); err != nil {
1530+
t.Fatalf("failed to create tasks dir: %v", err)
1531+
}
1532+
1533+
createStandardTask(t, tasksDir, "test-task")
1534+
1535+
// Run the program with local directory using file:// URL
1536+
localURL := "file://" + localDir
1537+
output := runTool(t, "-C", tmpDir, "-d", localURL, "test-task")
1538+
1539+
// Check that local rule content is present
1540+
if !strings.Contains(output, "# Local Rule") {
1541+
t.Errorf("local rule content not found in stdout")
1542+
}
1543+
if !strings.Contains(output, "This is a rule from a local directory") {
1544+
t.Errorf("local rule description not found in stdout")
1545+
}
1546+
1547+
// Verify the marker file still exists (directory was not deleted)
1548+
if _, err := os.Stat(markerFile); err != nil {
1549+
if os.IsNotExist(err) {
1550+
t.Errorf("marker file was deleted, indicating local directory was deleted")
1551+
} else {
1552+
t.Fatalf("unexpected error checking marker file: %v", err)
1553+
}
1554+
}
1555+
1556+
// Verify the rule file still exists
1557+
if _, err := os.Stat(ruleFile); err != nil {
1558+
if os.IsNotExist(err) {
1559+
t.Errorf("rule file was deleted, indicating local directory was deleted")
1560+
} else {
1561+
t.Fatalf("unexpected error checking rule file: %v", err)
1562+
}
1563+
}
1564+
}
1565+
1566+
// TestLocalDirectoryWithoutProtocol verifies that local directories passed
1567+
// without the file:// protocol are not deleted.
1568+
func TestLocalDirectoryWithoutProtocol(t *testing.T) {
1569+
// Create a local directory with a rule file and a marker file
1570+
localDir := t.TempDir()
1571+
rulesDir := filepath.Join(localDir, ".agents", "rules")
1572+
1573+
if err := os.MkdirAll(rulesDir, 0o755); err != nil {
1574+
t.Fatalf("failed to create rules dir: %v", err)
1575+
}
1576+
1577+
// Create a rule file
1578+
ruleFile := filepath.Join(rulesDir, "local-rule.md")
1579+
ruleContent := `---
1580+
language: go
1581+
---
1582+
# Local Rule
1583+
1584+
This is a rule from a local directory without protocol.
1585+
`
1586+
if err := os.WriteFile(ruleFile, []byte(ruleContent), 0o644); err != nil {
1587+
t.Fatalf("failed to write rule file: %v", err)
1588+
}
1589+
1590+
// Create a marker file to verify the directory is not deleted
1591+
markerFile := filepath.Join(localDir, "marker.txt")
1592+
if err := os.WriteFile(markerFile, []byte("marker"), 0o644); err != nil {
1593+
t.Fatalf("failed to write marker file: %v", err)
1594+
}
1595+
1596+
// Create a temporary directory for the task
1597+
tmpDir := t.TempDir()
1598+
tasksDir := filepath.Join(tmpDir, ".agents", "tasks")
1599+
1600+
if err := os.MkdirAll(tasksDir, 0o755); err != nil {
1601+
t.Fatalf("failed to create tasks dir: %v", err)
1602+
}
1603+
1604+
createStandardTask(t, tasksDir, "test-task")
1605+
1606+
// Run the program with local directory using absolute path (no protocol)
1607+
output := runTool(t, "-C", tmpDir, "-d", localDir, "test-task")
1608+
1609+
// Check that local rule content is present
1610+
if !strings.Contains(output, "# Local Rule") {
1611+
t.Errorf("local rule content not found in stdout")
1612+
}
1613+
if !strings.Contains(output, "This is a rule from a local directory without protocol") {
1614+
t.Errorf("local rule description not found in stdout")
1615+
}
1616+
1617+
// Verify the marker file still exists (directory was not deleted)
1618+
if _, err := os.Stat(markerFile); err != nil {
1619+
if os.IsNotExist(err) {
1620+
t.Errorf("marker file was deleted, indicating local directory was deleted")
1621+
} else {
1622+
t.Fatalf("unexpected error checking marker file: %v", err)
1623+
}
1624+
}
1625+
1626+
// Verify the rule file still exists
1627+
if _, err := os.Stat(ruleFile); err != nil {
1628+
if os.IsNotExist(err) {
1629+
t.Errorf("rule file was deleted, indicating local directory was deleted")
1630+
} else {
1631+
t.Fatalf("unexpected error checking rule file: %v", err)
1632+
}
1633+
}
1634+
}

pkg/codingcontext/context.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,37 @@ func (cc *Context) Run(ctx context.Context, taskName string) (*Result, error) {
348348
return result, nil
349349
}
350350

351+
// isLocalPath checks if a path is a local file system path.
352+
// Returns true for:
353+
// - file:// URLs (e.g., file:///path/to/dir)
354+
// - Absolute paths (e.g., /path/to/dir)
355+
// - Relative paths (e.g., ./path or ../path)
356+
// Returns false for remote protocols like git::, https://, s3::, etc.
357+
func isLocalPath(path string) bool {
358+
// Check if path starts with file:// protocol
359+
if strings.HasPrefix(path, "file://") {
360+
return true
361+
}
362+
363+
// Check if it's an absolute or relative local path
364+
// (no protocol prefix like git::, https://, s3::, etc.)
365+
if !strings.Contains(path, "://") && !strings.Contains(path, "::") {
366+
return true
367+
}
368+
369+
return false
370+
}
371+
372+
// normalizeLocalPath converts a local path to a usable file system path.
373+
// For file:// URLs, it strips the protocol prefix.
374+
// For other local paths, it returns them as-is.
375+
func normalizeLocalPath(path string) string {
376+
if strings.HasPrefix(path, "file://") {
377+
return strings.TrimPrefix(path, "file://")
378+
}
379+
return path
380+
}
381+
351382
func downloadDir(path string) string {
352383
// hash the path and prepend it with a temporary directory
353384
hash := sha256.Sum256([]byte(path))
@@ -397,6 +428,15 @@ func (cc *Context) parseManifestFile(ctx context.Context) ([]string, error) {
397428

398429
func (cc *Context) downloadRemoteDirectories(ctx context.Context) error {
399430
for _, path := range cc.searchPaths {
431+
// If the path is local, use it directly without downloading
432+
if isLocalPath(path) {
433+
localPath := normalizeLocalPath(path)
434+
cc.logger.Info("Using local directory", "path", localPath)
435+
cc.downloadedPaths = append(cc.downloadedPaths, localPath)
436+
continue
437+
}
438+
439+
// Download remote directories
400440
cc.logger.Info("Downloading remote directory", "path", path)
401441
dst := downloadDir(path)
402442
if _, err := getter.Get(ctx, dst, path); err != nil {
@@ -411,6 +451,12 @@ func (cc *Context) downloadRemoteDirectories(ctx context.Context) error {
411451

412452
func (cc *Context) cleanupDownloadedDirectories() {
413453
for _, path := range cc.searchPaths {
454+
// Skip cleanup for local paths - they should not be deleted
455+
if isLocalPath(path) {
456+
continue
457+
}
458+
459+
// Only clean up downloaded remote directories
414460
dst := downloadDir(path)
415461
if err := os.RemoveAll(dst); err != nil {
416462
cc.logger.Error("Error cleaning up downloaded directory", "path", dst, "error", err)

pkg/codingcontext/context_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,3 +1695,116 @@ func TestUserPrompt(t *testing.T) {
16951695
})
16961696
}
16971697
}
1698+
1699+
// TestIsLocalPath tests the isLocalPath helper function
1700+
func TestIsLocalPath(t *testing.T) {
1701+
tests := []struct {
1702+
name string
1703+
path string
1704+
expected bool
1705+
}{
1706+
{
1707+
name: "file:// protocol",
1708+
path: "file:///path/to/local",
1709+
expected: true,
1710+
},
1711+
{
1712+
name: "absolute path",
1713+
path: "/path/to/local",
1714+
expected: true,
1715+
},
1716+
{
1717+
name: "relative path - ./",
1718+
path: "./relative/path",
1719+
expected: true,
1720+
},
1721+
{
1722+
name: "relative path - ../",
1723+
path: "../relative/path",
1724+
expected: true,
1725+
},
1726+
{
1727+
name: "relative path - no prefix",
1728+
path: "relative/path",
1729+
expected: true,
1730+
},
1731+
{
1732+
name: "git protocol",
1733+
path: "git::https://github.com/user/repo.git",
1734+
expected: false,
1735+
},
1736+
{
1737+
name: "https protocol",
1738+
path: "https://example.com/file.tar.gz",
1739+
expected: false,
1740+
},
1741+
{
1742+
name: "http protocol",
1743+
path: "http://example.com/file.tar.gz",
1744+
expected: false,
1745+
},
1746+
{
1747+
name: "s3 protocol",
1748+
path: "s3::https://s3.amazonaws.com/bucket/key",
1749+
expected: false,
1750+
},
1751+
}
1752+
1753+
for _, tt := range tests {
1754+
t.Run(tt.name, func(t *testing.T) {
1755+
result := isLocalPath(tt.path)
1756+
if result != tt.expected {
1757+
t.Errorf("isLocalPath(%q) = %v, expected %v", tt.path, result, tt.expected)
1758+
}
1759+
})
1760+
}
1761+
}
1762+
1763+
// TestNormalizeLocalPath tests the normalizeLocalPath helper function
1764+
func TestNormalizeLocalPath(t *testing.T) {
1765+
tests := []struct {
1766+
name string
1767+
path string
1768+
expected string
1769+
}{
1770+
{
1771+
name: "file:// protocol - absolute path",
1772+
path: "file:///path/to/local",
1773+
expected: "/path/to/local",
1774+
},
1775+
{
1776+
name: "file:// protocol - relative path",
1777+
path: "file://./relative/path",
1778+
expected: "./relative/path",
1779+
},
1780+
{
1781+
name: "absolute path without protocol",
1782+
path: "/path/to/local",
1783+
expected: "/path/to/local",
1784+
},
1785+
{
1786+
name: "relative path - ./",
1787+
path: "./relative/path",
1788+
expected: "./relative/path",
1789+
},
1790+
{
1791+
name: "relative path - ../",
1792+
path: "../relative/path",
1793+
expected: "../relative/path",
1794+
},
1795+
{
1796+
name: "relative path - no prefix",
1797+
path: "relative/path",
1798+
expected: "relative/path",
1799+
},
1800+
}
1801+
1802+
for _, tt := range tests {
1803+
t.Run(tt.name, func(t *testing.T) {
1804+
result := normalizeLocalPath(tt.path)
1805+
if result != tt.expected {
1806+
t.Errorf("normalizeLocalPath(%q) = %q, expected %q", tt.path, result, tt.expected)
1807+
}
1808+
})
1809+
}
1810+
}

0 commit comments

Comments
 (0)