Skip to content

Commit 9399da4

Browse files
daniel-lxsdaniel-lxs
committed
refactor: remove unrelated changes from todo list settings PR
- Reverted unrelated symlink handling refactoring in custom-instructions.ts - Reverted unrelated test mocking refactoring in system-prompt.spec.ts - Kept only necessary changes for todo list feature: - settings parameter in addCustomInstructions interface - three test cases for todo list enable/disable functionality - updated mock to accept settings parameter
1 parent 97073c6 commit 9399da4

File tree

2 files changed

+92
-117
lines changed

2 files changed

+92
-117
lines changed

‎src/core/prompts/__tests__/system-prompt.spec.ts‎

Lines changed: 55 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -59,91 +59,65 @@ vi.mock("../sections/modes", () => ({
5959
getModesSection: vi.fn().mockImplementation(async () => `====\n\nMODES\n\n- Test modes section`),
6060
}))
6161

62-
// Mock fs/promises to prevent file system access in tests
63-
vi.mock("fs/promises", () => ({
64-
default: {
65-
readFile: vi.fn().mockRejectedValue({ code: "ENOENT" }),
66-
readdir: vi.fn().mockResolvedValue([]),
67-
stat: vi.fn().mockRejectedValue({ code: "ENOENT" }),
68-
readlink: vi.fn().mockRejectedValue({ code: "ENOENT" }),
69-
},
70-
readFile: vi.fn().mockRejectedValue({ code: "ENOENT" }),
71-
readdir: vi.fn().mockResolvedValue([]),
72-
stat: vi.fn().mockRejectedValue({ code: "ENOENT" }),
73-
readlink: vi.fn().mockRejectedValue({ code: "ENOENT" }),
74-
}))
75-
76-
// Conditionally mock custom instructions
77-
let useRealCustomInstructions = false
78-
79-
vi.mock("../sections/custom-instructions", async () => {
80-
const actual = await vi.importActual<typeof import("../sections/custom-instructions")>(
81-
"../sections/custom-instructions",
82-
)
83-
62+
// Mock the custom instructions
63+
vi.mock("../sections/custom-instructions", () => {
64+
const addCustomInstructions = vi.fn()
8465
return {
85-
...actual,
86-
addCustomInstructions: vi
87-
.fn()
88-
.mockImplementation(
89-
async (
90-
modeCustomInstructions: string,
91-
globalCustomInstructions: string,
92-
cwd: string,
93-
mode: string,
94-
options?: { language?: string; rooIgnoreInstructions?: string; settings?: Record<string, any> },
95-
) => {
96-
if (useRealCustomInstructions) {
97-
// Use the real implementation for todo list tests
98-
return actual.addCustomInstructions(
99-
modeCustomInstructions,
100-
globalCustomInstructions,
101-
cwd,
102-
mode,
103-
options,
104-
)
105-
}
106-
107-
// Use mock implementation for other tests
108-
const sections = []
109-
110-
// Add language preference if provided
111-
if (options?.language) {
112-
sections.push(
113-
`Language Preference:\nYou should always speak and think in the "${options.language}" language.`,
114-
)
115-
}
116-
117-
// Add global instructions first
118-
if (globalCustomInstructions?.trim()) {
119-
sections.push(`Global Instructions:\n${globalCustomInstructions.trim()}`)
120-
}
121-
122-
// Add mode-specific instructions after
123-
if (modeCustomInstructions?.trim()) {
124-
sections.push(`Mode-specific Instructions:\n${modeCustomInstructions}`)
125-
}
126-
127-
// Add rules
128-
const rules = []
129-
if (mode) {
130-
rules.push(`# Rules from .clinerules-${mode}:\nMock mode-specific rules`)
131-
}
132-
rules.push(`# Rules from .clinerules:\nMock generic rules`)
133-
134-
if (rules.length > 0) {
135-
sections.push(`Rules:\n${rules.join("\n")}`)
136-
}
137-
138-
const joinedSections = sections.join("\n\n")
139-
return joinedSections
140-
? `\n====\n\nUSER'S CUSTOM INSTRUCTIONS\n\nThe following additional instructions are provided by the user, and should be followed to the best of your ability without interfering with the TOOL USE guidelines.\n\n${joinedSections}`
141-
: ""
142-
},
143-
),
66+
addCustomInstructions,
67+
__setMockImplementation: (impl: any) => {
68+
addCustomInstructions.mockImplementation(impl)
69+
},
14470
}
14571
})
14672

73+
// Set up default mock implementation
74+
const customInstructionsMock = vi.mocked(await import("../sections/custom-instructions"))
75+
const { __setMockImplementation } = customInstructionsMock as any
76+
__setMockImplementation(
77+
async (
78+
modeCustomInstructions: string,
79+
globalCustomInstructions: string,
80+
cwd: string,
81+
mode: string,
82+
options?: { language?: string; rooIgnoreInstructions?: string; settings?: Record<string, any> },
83+
) => {
84+
const sections = []
85+
86+
// Add language preference if provided
87+
if (options?.language) {
88+
sections.push(
89+
`Language Preference:\nYou should always speak and think in the "${options.language}" language.`,
90+
)
91+
}
92+
93+
// Add global instructions first
94+
if (globalCustomInstructions?.trim()) {
95+
sections.push(`Global Instructions:\n${globalCustomInstructions.trim()}`)
96+
}
97+
98+
// Add mode-specific instructions after
99+
if (modeCustomInstructions?.trim()) {
100+
sections.push(`Mode-specific Instructions:\n${modeCustomInstructions}`)
101+
}
102+
103+
// Add rules
104+
const rules = []
105+
if (mode) {
106+
rules.push(`# Rules from .clinerules-${mode}:\nMock mode-specific rules`)
107+
}
108+
rules.push(`# Rules from .clinerules:\nMock generic rules`)
109+
110+
if (rules.length > 0) {
111+
sections.push(`Rules:\n${rules.join("\n")}`)
112+
}
113+
114+
const joinedSections = sections.join("\n\n")
115+
return joinedSections
116+
? `\n====\n\nUSER'S CUSTOM INSTRUCTIONS\n\nThe following additional instructions are provided by the user, and should be followed to the best of your ability without interfering with the TOOL USE guidelines.\n\n${joinedSections}`
117+
: ""
118+
},
119+
)
120+
147121
// Mock vscode language
148122
vi.mock("vscode", () => ({
149123
env: {
@@ -602,9 +576,6 @@ describe("SYSTEM_PROMPT", () => {
602576
})
603577

604578
it("should exclude update_todo_list tool when todoListEnabled is false", async () => {
605-
// Use real custom instructions implementation for this test
606-
useRealCustomInstructions = true
607-
608579
const settings = {
609580
todoListEnabled: false,
610581
}
@@ -632,15 +603,9 @@ describe("SYSTEM_PROMPT", () => {
632603
// Should not contain the tool description
633604
expect(prompt).not.toContain("## update_todo_list")
634605
// Mode instructions will still reference the tool with a fallback to markdown
635-
636-
// Reset flag
637-
useRealCustomInstructions = false
638606
})
639607

640608
it("should include update_todo_list tool when todoListEnabled is true", async () => {
641-
// Use real custom instructions implementation for this test
642-
useRealCustomInstructions = true
643-
644609
const settings = {
645610
todoListEnabled: true,
646611
}
@@ -667,15 +632,9 @@ describe("SYSTEM_PROMPT", () => {
667632

668633
expect(prompt).toContain("update_todo_list")
669634
expect(prompt).toContain("## update_todo_list")
670-
671-
// Reset flag
672-
useRealCustomInstructions = false
673635
})
674636

675637
it("should include update_todo_list tool when todoListEnabled is undefined", async () => {
676-
// Use real custom instructions implementation for this test
677-
useRealCustomInstructions = true
678-
679638
const settings = {
680639
// todoListEnabled not set
681640
}
@@ -702,9 +661,6 @@ describe("SYSTEM_PROMPT", () => {
702661

703662
expect(prompt).toContain("update_todo_list")
704663
expect(prompt).toContain("## update_todo_list")
705-
706-
// Reset flag
707-
useRealCustomInstructions = false
708664
})
709665

710666
afterAll(() => {

‎src/core/prompts/sections/custom-instructions.ts‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const MAX_DEPTH = 5
4444
async function resolveDirectoryEntry(
4545
entry: Dirent,
4646
dirPath: string,
47-
filePaths: string[],
47+
fileInfo: Array<{ originalPath: string; resolvedPath: string }>,
4848
depth: number,
4949
): Promise<void> {
5050
// Avoid cyclic symlinks
@@ -54,44 +54,49 @@ async function resolveDirectoryEntry(
5454

5555
const fullPath = path.resolve(entry.parentPath || dirPath, entry.name)
5656
if (entry.isFile()) {
57-
// Regular file
58-
filePaths.push(fullPath)
57+
// Regular file - both original and resolved paths are the same
58+
fileInfo.push({ originalPath: fullPath, resolvedPath: fullPath })
5959
} else if (entry.isSymbolicLink()) {
6060
// Await the resolution of the symbolic link
61-
await resolveSymLink(fullPath, filePaths, depth + 1)
61+
await resolveSymLink(fullPath, fileInfo, depth + 1)
6262
}
6363
}
6464

6565
/**
6666
* Recursively resolve a symbolic link and collect file paths
6767
*/
68-
async function resolveSymLink(fullPath: string, filePaths: string[], depth: number): Promise<void> {
68+
async function resolveSymLink(
69+
symlinkPath: string,
70+
fileInfo: Array<{ originalPath: string; resolvedPath: string }>,
71+
depth: number,
72+
): Promise<void> {
6973
// Avoid cyclic symlinks
7074
if (depth > MAX_DEPTH) {
7175
return
7276
}
7377
try {
7478
// Get the symlink target
75-
const linkTarget = await fs.readlink(fullPath)
79+
const linkTarget = await fs.readlink(symlinkPath)
7680
// Resolve the target path (relative to the symlink location)
77-
const resolvedTarget = path.resolve(path.dirname(fullPath), linkTarget)
81+
const resolvedTarget = path.resolve(path.dirname(symlinkPath), linkTarget)
7882

7983
// Check if the target is a file
8084
const stats = await fs.stat(resolvedTarget)
8185
if (stats.isFile()) {
82-
filePaths.push(resolvedTarget)
86+
// For symlinks to files, store the symlink path as original and target as resolved
87+
fileInfo.push({ originalPath: symlinkPath, resolvedPath: resolvedTarget })
8388
} else if (stats.isDirectory()) {
8489
const anotherEntries = await fs.readdir(resolvedTarget, { withFileTypes: true, recursive: true })
8590
// Collect promises for recursive calls within the directory
8691
const directoryPromises: Promise<void>[] = []
8792
for (const anotherEntry of anotherEntries) {
88-
directoryPromises.push(resolveDirectoryEntry(anotherEntry, resolvedTarget, filePaths, depth + 1))
93+
directoryPromises.push(resolveDirectoryEntry(anotherEntry, resolvedTarget, fileInfo, depth + 1))
8994
}
9095
// Wait for all entries in the resolved directory to be processed
9196
await Promise.all(directoryPromises)
9297
} else if (stats.isSymbolicLink()) {
9398
// Handle nested symlinks by awaiting the recursive call
94-
await resolveSymLink(resolvedTarget, filePaths, depth + 1)
99+
await resolveSymLink(resolvedTarget, fileInfo, depth + 1)
95100
}
96101
} catch (err) {
97102
// Skip invalid symlinks
@@ -106,29 +111,31 @@ async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ file
106111
const entries = await fs.readdir(dirPath, { withFileTypes: true, recursive: true })
107112

108113
// Process all entries - regular files and symlinks that might point to files
109-
const filePaths: string[] = []
114+
// Store both original path (for sorting) and resolved path (for reading)
115+
const fileInfo: Array<{ originalPath: string; resolvedPath: string }> = []
110116
// Collect promises for the initial resolution calls
111117
const initialPromises: Promise<void>[] = []
112118

113119
for (const entry of entries) {
114-
initialPromises.push(resolveDirectoryEntry(entry, dirPath, filePaths, 0))
120+
initialPromises.push(resolveDirectoryEntry(entry, dirPath, fileInfo, 0))
115121
}
116122

117123
// Wait for all asynchronous operations (including recursive ones) to complete
118124
await Promise.all(initialPromises)
119125

120126
const fileContents = await Promise.all(
121-
filePaths.map(async (file) => {
127+
fileInfo.map(async ({ originalPath, resolvedPath }) => {
122128
try {
123129
// Check if it's a file (not a directory)
124-
const stats = await fs.stat(file)
130+
const stats = await fs.stat(resolvedPath)
125131
if (stats.isFile()) {
126132
// Filter out cache files and system files that shouldn't be in rules
127-
if (!shouldIncludeRuleFile(file)) {
133+
if (!shouldIncludeRuleFile(resolvedPath)) {
128134
return null
129135
}
130-
const content = await safeReadFile(file)
131-
return { filename: file, content }
136+
const content = await safeReadFile(resolvedPath)
137+
// Use resolvedPath for display to maintain existing behavior
138+
return { filename: resolvedPath, content, sortKey: originalPath }
132139
}
133140
return null
134141
} catch (err) {
@@ -138,7 +145,19 @@ async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ file
138145
)
139146

140147
// Filter out null values (directories, failed reads, or excluded files)
141-
return fileContents.filter((item): item is { filename: string; content: string } => item !== null)
148+
const filteredFiles = fileContents.filter(
149+
(item): item is { filename: string; content: string; sortKey: string } => item !== null,
150+
)
151+
152+
// Sort files alphabetically by the original filename (case-insensitive) to ensure consistent order
153+
// For symlinks, this will use the symlink name, not the target name
154+
return filteredFiles
155+
.sort((a, b) => {
156+
const filenameA = path.basename(a.sortKey).toLowerCase()
157+
const filenameB = path.basename(b.sortKey).toLowerCase()
158+
return filenameA.localeCompare(filenameB)
159+
})
160+
.map(({ filename, content }) => ({ filename, content }))
142161
} catch (err) {
143162
return []
144163
}

0 commit comments

Comments
 (0)