Merge pull request #2396 from code-yeongyu/fix/lsp-directory-diagnostics-followup
fix(lsp): make directory diagnostics output actionable
This commit is contained in:
@@ -184,7 +184,7 @@ task(
|
|||||||
After EVERY delegation, complete ALL of these steps — no shortcuts:
|
After EVERY delegation, complete ALL of these steps — no shortcuts:
|
||||||
|
|
||||||
#### A. Automated Verification
|
#### A. Automated Verification
|
||||||
1. 'lsp_diagnostics(filePath=".", extension=".ts")' → ZERO errors at project level (for directory paths, extension parameter is required)
|
1. 'lsp_diagnostics(filePath=".", extension=".ts")' → ZERO errors across scanned TypeScript files (directory scans are capped at 50 files; not a full-project guarantee)
|
||||||
2. \`bun run build\` or \`bun run typecheck\` → exit code 0
|
2. \`bun run build\` or \`bun run typecheck\` → exit code 0
|
||||||
3. \`bun test\` → ALL tests pass
|
3. \`bun test\` → ALL tests pass
|
||||||
|
|
||||||
@@ -346,7 +346,7 @@ You are the QA gate. Subagents lie. Verify EVERYTHING.
|
|||||||
|
|
||||||
**After each delegation — BOTH automated AND manual verification are MANDATORY:**
|
**After each delegation — BOTH automated AND manual verification are MANDATORY:**
|
||||||
|
|
||||||
1. 'lsp_diagnostics(filePath=".", extension=".ts")' at PROJECT level → ZERO errors (for directory paths, extension parameter is required)
|
1. 'lsp_diagnostics(filePath=".", extension=".ts")' across scanned TypeScript files → ZERO errors (directory scans are capped at 50 files; not a full-project guarantee)
|
||||||
2. Run build command → exit 0
|
2. Run build command → exit 0
|
||||||
3. Run test suite → ALL pass
|
3. Run test suite → ALL pass
|
||||||
4. **\`Read\` EVERY changed file line by line** → logic matches requirements
|
4. **\`Read\` EVERY changed file line by line** → logic matches requirements
|
||||||
@@ -390,14 +390,14 @@ You are the QA gate. Subagents lie. Verify EVERYTHING.
|
|||||||
- Trust subagent claims without verification
|
- Trust subagent claims without verification
|
||||||
- Use run_in_background=true for task execution
|
- Use run_in_background=true for task execution
|
||||||
- Send prompts under 30 lines
|
- Send prompts under 30 lines
|
||||||
- Skip project-level lsp_diagnostics after delegation (use 'filePath=".", extension=".ts"' for TypeScript projects)
|
- Skip scanned-file lsp_diagnostics after delegation (use 'filePath=".", extension=".ts"' for TypeScript projects; directory scans are capped at 50 files)
|
||||||
- Batch multiple tasks in one delegation
|
- Batch multiple tasks in one delegation
|
||||||
- Start fresh session for failures/follow-ups - use \`resume\` instead
|
- Start fresh session for failures/follow-ups - use \`resume\` instead
|
||||||
|
|
||||||
**ALWAYS**:
|
**ALWAYS**:
|
||||||
- Include ALL 6 sections in delegation prompts
|
- Include ALL 6 sections in delegation prompts
|
||||||
- Read notepad before every delegation
|
- Read notepad before every delegation
|
||||||
- Run project-level QA after every delegation
|
- Run scanned-file QA after every delegation
|
||||||
- Pass inherited wisdom to every subagent
|
- Pass inherited wisdom to every subagent
|
||||||
- Parallelize independent tasks
|
- Parallelize independent tasks
|
||||||
- Verify with your own tools
|
- Verify with your own tools
|
||||||
|
|||||||
@@ -361,14 +361,14 @@ Subagents CLAIM "done" when:
|
|||||||
- Trust subagent claims without verification
|
- Trust subagent claims without verification
|
||||||
- Use run_in_background=true for task execution
|
- Use run_in_background=true for task execution
|
||||||
- Send prompts under 30 lines
|
- Send prompts under 30 lines
|
||||||
- Skip project-level lsp_diagnostics (use 'filePath=".", extension=".ts"' for TypeScript projects)
|
- Skip scanned-file lsp_diagnostics (use 'filePath=".", extension=".ts"' for TypeScript projects; directory scans are capped at 50 files)
|
||||||
- Batch multiple tasks in one delegation
|
- Batch multiple tasks in one delegation
|
||||||
- Start fresh session for failures (use session_id)
|
- Start fresh session for failures (use session_id)
|
||||||
|
|
||||||
**ALWAYS**:
|
**ALWAYS**:
|
||||||
- Include ALL 6 sections in delegation prompts
|
- Include ALL 6 sections in delegation prompts
|
||||||
- Read notepad before every delegation
|
- Read notepad before every delegation
|
||||||
- Run project-level QA after every delegation
|
- Run scanned-file QA after every delegation
|
||||||
- Pass inherited wisdom to every subagent
|
- Pass inherited wisdom to every subagent
|
||||||
- Parallelize independent tasks
|
- Parallelize independent tasks
|
||||||
- Store and reuse session_id for retries
|
- Store and reuse session_id for retries
|
||||||
@@ -392,4 +392,4 @@ This ensures accurate progress tracking. Skip this and you lose visibility into
|
|||||||
|
|
||||||
export function getGeminiAtlasPrompt(): string {
|
export function getGeminiAtlasPrompt(): string {
|
||||||
return ATLAS_GEMINI_SYSTEM_PROMPT
|
return ATLAS_GEMINI_SYSTEM_PROMPT
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ Implementation tasks are the means. Final Wave approval is the goal.
|
|||||||
- Verification (use Bash for tests/build)
|
- Verification (use Bash for tests/build)
|
||||||
- Parallelize independent tool calls when possible.
|
- Parallelize independent tool calls when possible.
|
||||||
- After ANY delegation, verify with your own tool calls:
|
- After ANY delegation, verify with your own tool calls:
|
||||||
1. 'lsp_diagnostics(filePath=".", extension=".ts")' at project level (for directory paths, extension parameter is required)
|
1. 'lsp_diagnostics(filePath=".", extension=".ts")' across scanned TypeScript files (directory scans are capped at 50 files; not a full-project guarantee)
|
||||||
2. \`Bash\` for build/test commands
|
2. \`Bash\` for build/test commands
|
||||||
3. \`Read\` for changed files
|
3. \`Read\` for changed files
|
||||||
</tool_usage_rules>
|
</tool_usage_rules>
|
||||||
@@ -364,14 +364,14 @@ Your job is to CATCH THEM. Assume every claim is false until YOU personally veri
|
|||||||
- Trust subagent claims without verification
|
- Trust subagent claims without verification
|
||||||
- Use run_in_background=true for task execution
|
- Use run_in_background=true for task execution
|
||||||
- Send prompts under 30 lines
|
- Send prompts under 30 lines
|
||||||
- Skip project-level lsp_diagnostics (use 'filePath=".", extension=".ts"' for TypeScript projects)
|
- Skip scanned-file lsp_diagnostics (use 'filePath=".", extension=".ts"' for TypeScript projects; directory scans are capped at 50 files)
|
||||||
- Batch multiple tasks in one delegation
|
- Batch multiple tasks in one delegation
|
||||||
- Start fresh session for failures (use session_id)
|
- Start fresh session for failures (use session_id)
|
||||||
|
|
||||||
**ALWAYS**:
|
**ALWAYS**:
|
||||||
- Include ALL 6 sections in delegation prompts
|
- Include ALL 6 sections in delegation prompts
|
||||||
- Read notepad before every delegation
|
- Read notepad before every delegation
|
||||||
- Run project-level QA after every delegation
|
- Run scanned-file QA after every delegation
|
||||||
- Pass inherited wisdom to every subagent
|
- Pass inherited wisdom to every subagent
|
||||||
- Parallelize independent tasks
|
- Parallelize independent tasks
|
||||||
- Store and reuse session_id for retries
|
- Store and reuse session_id for retries
|
||||||
|
|||||||
@@ -1,12 +1,53 @@
|
|||||||
import { describe, expect, it } from "bun:test"
|
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"
|
||||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"
|
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "fs"
|
||||||
import { join } from "path"
|
import { join } from "path"
|
||||||
import os from "os"
|
import os from "os"
|
||||||
|
|
||||||
|
import * as configModule from "./config"
|
||||||
|
import { lspManager } from "./lsp-server"
|
||||||
import { isDirectoryPath } from "./lsp-client-wrapper"
|
import { isDirectoryPath } from "./lsp-client-wrapper"
|
||||||
import { aggregateDiagnosticsForDirectory } from "./directory-diagnostics"
|
import { aggregateDiagnosticsForDirectory } from "./directory-diagnostics"
|
||||||
|
import type { Diagnostic } from "./types"
|
||||||
|
|
||||||
|
const diagnosticsMock = mock(async (_filePath: string) => ({ items: [] as Diagnostic[] }))
|
||||||
|
const getClientMock = mock(async () => ({ diagnostics: diagnosticsMock }))
|
||||||
|
const releaseClientMock = mock(() => {})
|
||||||
|
|
||||||
|
function createDiagnostic(message: string): Diagnostic {
|
||||||
|
return {
|
||||||
|
message,
|
||||||
|
severity: 1,
|
||||||
|
range: {
|
||||||
|
start: { line: 0, character: 0 },
|
||||||
|
end: { line: 0, character: 1 },
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
describe("directory diagnostics", () => {
|
describe("directory diagnostics", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
diagnosticsMock.mockReset()
|
||||||
|
diagnosticsMock.mockImplementation(async (_filePath: string) => ({ items: [] }))
|
||||||
|
getClientMock.mockClear()
|
||||||
|
releaseClientMock.mockClear()
|
||||||
|
|
||||||
|
spyOn(configModule, "findServerForExtension").mockReturnValue({
|
||||||
|
status: "found",
|
||||||
|
server: {
|
||||||
|
id: "test-server",
|
||||||
|
command: ["test-server"],
|
||||||
|
extensions: [".ts"],
|
||||||
|
priority: 1,
|
||||||
|
},
|
||||||
|
})
|
||||||
|
spyOn(lspManager, "getClient").mockImplementation(getClientMock)
|
||||||
|
spyOn(lspManager, "releaseClient").mockImplementation(releaseClientMock)
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
mock.restore()
|
||||||
|
})
|
||||||
|
|
||||||
describe("isDirectoryPath", () => {
|
describe("isDirectoryPath", () => {
|
||||||
it("returns true for existing directory", () => {
|
it("returns true for existing directory", () => {
|
||||||
const tmp = mkdtempSync(join(os.tmpdir(), "omo-isdir-"))
|
const tmp = mkdtempSync(join(os.tmpdir(), "omo-isdir-"))
|
||||||
@@ -52,5 +93,27 @@ describe("directory diagnostics", () => {
|
|||||||
"Directory does not exist"
|
"Directory does not exist"
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("#given diagnostics from multiple files #when aggregating directory diagnostics #then each entry includes the source file path", async () => {
|
||||||
|
const tmp = mkdtempSync(join(os.tmpdir(), "omo-aggr-files-"))
|
||||||
|
try {
|
||||||
|
const firstFile = join(tmp, "first.ts")
|
||||||
|
const secondFile = join(tmp, "second.ts")
|
||||||
|
|
||||||
|
writeFileSync(firstFile, "export const first = true\n")
|
||||||
|
writeFileSync(secondFile, "export const second = true\n")
|
||||||
|
|
||||||
|
diagnosticsMock.mockImplementation(async (filePath: string) => ({
|
||||||
|
items: [createDiagnostic(`problem in ${filePath}`)],
|
||||||
|
}))
|
||||||
|
|
||||||
|
const result = await aggregateDiagnosticsForDirectory(tmp, ".ts")
|
||||||
|
|
||||||
|
expect(result).toContain(`${firstFile}: error at 1:0: problem in ${firstFile}`)
|
||||||
|
expect(result).toContain(`${secondFile}: error at 1:0: problem in ${secondFile}`)
|
||||||
|
} finally {
|
||||||
|
rmSync(tmp, { recursive: true, force: true })
|
||||||
|
}
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -11,6 +11,11 @@ import type { Diagnostic } from "./types"
|
|||||||
|
|
||||||
const SKIP_DIRECTORIES = new Set(["node_modules", ".git", "dist", "build", ".next", "out"])
|
const SKIP_DIRECTORIES = new Set(["node_modules", ".git", "dist", "build", ".next", "out"])
|
||||||
|
|
||||||
|
type FileDiagnostic = {
|
||||||
|
filePath: string
|
||||||
|
diagnostic: Diagnostic
|
||||||
|
}
|
||||||
|
|
||||||
function collectFilesWithExtension(dir: string, extension: string, maxFiles: number): string[] {
|
function collectFilesWithExtension(dir: string, extension: string, maxFiles: number): string[] {
|
||||||
const files: string[] = []
|
const files: string[] = []
|
||||||
|
|
||||||
@@ -95,7 +100,7 @@ export async function aggregateDiagnosticsForDirectory(
|
|||||||
|
|
||||||
const root = findWorkspaceRoot(absDir)
|
const root = findWorkspaceRoot(absDir)
|
||||||
|
|
||||||
const allDiagnostics: Diagnostic[] = []
|
const allDiagnostics: FileDiagnostic[] = []
|
||||||
const fileErrors: { file: string; error: string }[] = []
|
const fileErrors: { file: string; error: string }[] = []
|
||||||
|
|
||||||
let client: LSPClient
|
let client: LSPClient
|
||||||
@@ -106,7 +111,12 @@ export async function aggregateDiagnosticsForDirectory(
|
|||||||
try {
|
try {
|
||||||
const result = await client.diagnostics(file)
|
const result = await client.diagnostics(file)
|
||||||
const filtered = filterDiagnosticsBySeverity(result.items, severity)
|
const filtered = filterDiagnosticsBySeverity(result.items, severity)
|
||||||
allDiagnostics.push(...filtered)
|
allDiagnostics.push(
|
||||||
|
...filtered.map((diagnostic) => ({
|
||||||
|
filePath: file,
|
||||||
|
diagnostic,
|
||||||
|
}))
|
||||||
|
)
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
fileErrors.push({
|
fileErrors.push({
|
||||||
file,
|
file,
|
||||||
@@ -138,8 +148,8 @@ export async function aggregateDiagnosticsForDirectory(
|
|||||||
|
|
||||||
if (displayDiagnostics.length > 0) {
|
if (displayDiagnostics.length > 0) {
|
||||||
lines.push("")
|
lines.push("")
|
||||||
for (const diag of displayDiagnostics) {
|
for (const { filePath, diagnostic } of displayDiagnostics) {
|
||||||
lines.push(formatDiagnostic(diag))
|
lines.push(`${filePath}: ${formatDiagnostic(diagnostic)}`)
|
||||||
}
|
}
|
||||||
if (wasDiagCapped) {
|
if (wasDiagCapped) {
|
||||||
lines.push(
|
lines.push(
|
||||||
|
|||||||
Reference in New Issue
Block a user