From e5c024e3133a75235b55a78f0d9d96c4fd454516 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Mon, 20 Jul 2026 22:10:31 +0530 Subject: [PATCH 1/2] feat: add Swift/PHP/Kotlin/Shell post-edit diagnostics Extend PATH-gated diagnostics with swiftlint, phpstan/pint/php -l, ktlint, and shellcheck so more languages get in-loop lint feedback after edits without auto-installing tools. --- README.md | 2 +- src/tools/diagnostics.test.ts | 106 ++++++++++++++++++++++++++++++++++ src/tools/diagnostics.ts | 19 ++++++ 3 files changed, 126 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 16968a7..d4ee499 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ The router escalates automatically when a task turns out harder than it looked a ### Post-Edit Diagnostics — the Model Sees Its Own Mistakes -After a successful edit, Klaat Code runs your project's typechecker/linter on the changed file (auto-detects eslint/biome, ruff, gofmt, or a configured command) and hands any errors straight back to the model in the same turn — it fixes them before returning control to you, instead of costing you a round-trip. +After a successful edit, Klaat Code runs your project's typechecker/linter on the changed file (auto-detects eslint/biome, ruff, gofmt, rubocop, swiftlint, phpstan/pint, ktlint, shellcheck, or a configured command) and hands any errors straight back to the model in the same turn — it fixes them before returning control to you, instead of costing you a round-trip. ### Terminal UI with Real Syntax Highlighting diff --git a/src/tools/diagnostics.test.ts b/src/tools/diagnostics.test.ts index 4b6fa95..40b9bd1 100644 --- a/src/tools/diagnostics.test.ts +++ b/src/tools/diagnostics.test.ts @@ -51,3 +51,109 @@ describe("Ruby diagnostics (.rb)", () => { expect(result === null || typeof result === "string").toBe(true); }); }); + +/** Shared shape for PATH-gated language branches: never throw; null or string. */ +function expectSoftResult(absPath: string): void { + expect(() => runDiagnostics(absPath, tmp)).not.toThrow(); + const result = runDiagnostics(absPath, tmp); + expect(result === null || typeof result === "string").toBe(true); +} + +describe("Swift diagnostics (.swift)", () => { + test("never throws; skips when swiftlint absent", () => { + const absPath = join(tmp, "Foo.swift"); + writeFileSync(absPath, "struct Foo {}\n"); + expectSoftResult(absPath); + }); + + test("returns null when diagnostics disabled", () => { + configureDiagnostics({ enabled: false }); + const absPath = join(tmp, "Bar.swift"); + writeFileSync(absPath, "struct Bar {}\n"); + expect(runDiagnostics(absPath, tmp)).toBeNull(); + }); +}); + +describe("PHP diagnostics (.php)", () => { + test("never throws; skips when phpstan/pint/php absent", () => { + const absPath = join(tmp, "foo.php"); + writeFileSync(absPath, " { + configureDiagnostics({ enabled: false }); + const absPath = join(tmp, "bar.php"); + writeFileSync(absPath, " { + test("never throws for .kt when ktlint absent", () => { + const absPath = join(tmp, "Foo.kt"); + writeFileSync(absPath, "fun main() {}\n"); + expectSoftResult(absPath); + }); + + test("never throws for .kts when ktlint absent", () => { + const absPath = join(tmp, "build.kts"); + writeFileSync(absPath, "plugins {}\n"); + expectSoftResult(absPath); + }); + + test("returns null when diagnostics disabled", () => { + configureDiagnostics({ enabled: false }); + const absPath = join(tmp, "Bar.kt"); + writeFileSync(absPath, "class Bar\n"); + expect(runDiagnostics(absPath, tmp)).toBeNull(); + }); +}); + +describe("Shell diagnostics (.sh / .bash / .zsh)", () => { + test("never throws for .sh when shellcheck absent", () => { + const absPath = join(tmp, "script.sh"); + writeFileSync(absPath, "#!/bin/sh\necho hi\n"); + expectSoftResult(absPath); + }); + + test("never throws for .bash when shellcheck absent", () => { + const absPath = join(tmp, "script.bash"); + writeFileSync(absPath, "#!/bin/bash\necho hi\n"); + expectSoftResult(absPath); + }); + + test("returns null when diagnostics disabled", () => { + configureDiagnostics({ enabled: false }); + const absPath = join(tmp, "x.sh"); + writeFileSync(absPath, "#!/bin/sh\ntrue\n"); + expect(runDiagnostics(absPath, tmp)).toBeNull(); + }); +}); + +describe("config override still wins for new extensions", () => { + test("explicit commands override PATH detection for .sh", () => { + configureDiagnostics({ + enabled: true, + timeoutMs: 8_000, + commands: { ".sh": "echo 'script.sh:1:1: error: fake' >&2; exit 1" }, + }); + const absPath = join(tmp, "override.sh"); + writeFileSync(absPath, "#!/bin/sh\n"); + const result = runDiagnostics(absPath, tmp); + expect(result).not.toBeNull(); + expect(result!).toContain("Diagnostics after this edit"); + expect(result!).toContain("fake"); + }); + + test("explicit clean override returns null for .swift", () => { + configureDiagnostics({ + enabled: true, + timeoutMs: 8_000, + commands: { ".swift": "true" }, + }); + const absPath = join(tmp, "clean.swift"); + writeFileSync(absPath, "struct Ok {}\n"); + expect(runDiagnostics(absPath, tmp)).toBeNull(); + }); +}); diff --git a/src/tools/diagnostics.ts b/src/tools/diagnostics.ts index 46ee499..90960f9 100644 --- a/src/tools/diagnostics.ts +++ b/src/tools/diagnostics.ts @@ -73,6 +73,25 @@ function commandFor(absPath: string, projectRoot: string): string[] | null { if (onPath("rubocop")) return ["rubocop", "--format", "emacs", "--no-color", absPath]; return null; } + if (ext === ".swift") { + if (onPath("swiftlint")) return ["swiftlint", "lint", "--quiet", "--reporter", "xcode", absPath]; + return null; + } + if (ext === ".php") { + // Prefer static analysis; fall back to Pint style check, then php -l syntax. + if (onPath("phpstan")) return ["phpstan", "analyse", "--no-progress", "--error-format=raw", absPath]; + if (onPath("pint")) return ["pint", "--test", absPath]; + if (onPath("php")) return ["php", "-l", absPath]; + return null; + } + if (ext === ".kt" || ext === ".kts") { + if (onPath("ktlint")) return ["ktlint", "--reporter=plain", absPath]; + return null; + } + if (ext === ".sh" || ext === ".bash" || ext === ".zsh") { + if (onPath("shellcheck")) return ["shellcheck", "-f", "gcc", absPath]; + return null; + } if (ext === ".rs") { // cargo check is whole-crate/slow — only via explicit config override. return null; From 0fdfd1e1649d82db771e085452719bee881d6ca3 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Mon, 20 Jul 2026 22:41:01 +0530 Subject: [PATCH 2/2] fix: address review bot feedback on diagnostics Drop .zsh from shellcheck (unsupported), remove php -l fallback, simplify ktlint argv for 1.x compatibility, and add positive-path tests via injectable PATH stubs on resolveDiagnosticsCommand. --- src/tools/diagnostics.test.ts | 57 +++++++++++++++++++++++++++++++++-- src/tools/diagnostics.ts | 35 +++++++++++++++------ 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/src/tools/diagnostics.test.ts b/src/tools/diagnostics.test.ts index 40b9bd1..2d66267 100644 --- a/src/tools/diagnostics.test.ts +++ b/src/tools/diagnostics.test.ts @@ -2,7 +2,11 @@ import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { runDiagnostics, configureDiagnostics } from "./diagnostics"; +import { + runDiagnostics, + configureDiagnostics, + resolveDiagnosticsCommand, +} from "./diagnostics"; let tmp: string; @@ -75,7 +79,7 @@ describe("Swift diagnostics (.swift)", () => { }); describe("PHP diagnostics (.php)", () => { - test("never throws; skips when phpstan/pint/php absent", () => { + test("never throws; skips when phpstan/pint absent", () => { const absPath = join(tmp, "foo.php"); writeFileSync(absPath, " { }); }); -describe("Shell diagnostics (.sh / .bash / .zsh)", () => { +describe("Shell diagnostics (.sh / .bash)", () => { test("never throws for .sh when shellcheck absent", () => { const absPath = join(tmp, "script.sh"); writeFileSync(absPath, "#!/bin/sh\necho hi\n"); @@ -123,6 +127,13 @@ describe("Shell diagnostics (.sh / .bash / .zsh)", () => { expectSoftResult(absPath); }); + test("does not run shellcheck on .zsh (unsupported)", () => { + const absPath = join(tmp, "script.zsh"); + writeFileSync(absPath, "#!/bin/zsh\necho hi\n"); + const cmd = resolveDiagnosticsCommand(absPath, tmp, { onPath: () => true }); + expect(cmd).toBeNull(); + }); + test("returns null when diagnostics disabled", () => { configureDiagnostics({ enabled: false }); const absPath = join(tmp, "x.sh"); @@ -131,6 +142,46 @@ describe("Shell diagnostics (.sh / .bash / .zsh)", () => { }); }); +describe("resolveDiagnosticsCommand — positive PATH stubs", () => { + const present = (name: string) => (cmd: string) => cmd === name; + + test("swiftlint argv", () => { + const abs = join(tmp, "A.swift"); + const cmd = resolveDiagnosticsCommand(abs, tmp, { onPath: present("swiftlint") }); + expect(cmd).toEqual(["swiftlint", "lint", "--quiet", "--reporter", "xcode", abs]); + }); + + test("phpstan preferred over pint", () => { + const abs = join(tmp, "a.php"); + const cmd = resolveDiagnosticsCommand(abs, tmp, { + onPath: (c) => c === "phpstan" || c === "pint", + }); + expect(cmd![0]).toBe("phpstan"); + expect(cmd).toContain(abs); + }); + + test("pint used when phpstan absent; php alone is not enough", () => { + const abs = join(tmp, "a.php"); + expect(resolveDiagnosticsCommand(abs, tmp, { onPath: present("pint") })![0]).toBe("pint"); + expect(resolveDiagnosticsCommand(abs, tmp, { onPath: present("php") })).toBeNull(); + }); + + test("ktlint argv has no --reporter flag", () => { + const abs = join(tmp, "A.kt"); + expect(resolveDiagnosticsCommand(abs, tmp, { onPath: present("ktlint") })) + .toEqual(["ktlint", abs]); + }); + + test("shellcheck argv for .sh and .bash", () => { + const sh = join(tmp, "a.sh"); + const bash = join(tmp, "a.bash"); + expect(resolveDiagnosticsCommand(sh, tmp, { onPath: present("shellcheck") })) + .toEqual(["shellcheck", "-f", "gcc", sh]); + expect(resolveDiagnosticsCommand(bash, tmp, { onPath: present("shellcheck") })) + .toEqual(["shellcheck", "-f", "gcc", bash]); + }); +}); + describe("config override still wins for new extensions", () => { test("explicit commands override PATH detection for .sh", () => { configureDiagnostics({ diff --git a/src/tools/diagnostics.ts b/src/tools/diagnostics.ts index 90960f9..6e87431 100644 --- a/src/tools/diagnostics.ts +++ b/src/tools/diagnostics.ts @@ -35,15 +35,29 @@ function hasLocalBin(projectRoot: string, bin: string): boolean { } /** Is a command available on PATH? */ -function onPath(cmd: string): boolean { +function onPathDefault(cmd: string): boolean { try { const r = spawnSync(process.platform === "win32" ? "where" : "which", [cmd], { timeout: 2000 }); return r.status === 0; } catch { return false; } } -/** Choose a fast per-file diagnostics command for a file, or null. */ -function commandFor(absPath: string, projectRoot: string): string[] | null { +export interface CommandResolveDeps { + onPath?: (cmd: string) => boolean; + hasLocalBin?: (projectRoot: string, bin: string) => boolean; +} + +/** + * Choose a fast per-file diagnostics command for a file, or null. + * Exported so tests can inject PATH/local-bin stubs and assert argv shape. + */ +export function resolveDiagnosticsCommand( + absPath: string, + projectRoot: string, + deps: CommandResolveDeps = {}, +): string[] | null { + const onPath = deps.onPath ?? onPathDefault; + const localBin = deps.hasLocalBin ?? hasLocalBin; const ext = extname(absPath).toLowerCase(); // Explicit config override wins. @@ -52,10 +66,10 @@ function commandFor(absPath: string, projectRoot: string): string[] | null { if ([".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"].includes(ext)) { // Prefer a locally-installed linter (fast, per-file). Never npx-install. - if (hasLocalBin(projectRoot, "eslint")) { + if (localBin(projectRoot, "eslint")) { return [join(projectRoot, "node_modules", ".bin", "eslint"), "--no-color", "--format", "unix", absPath]; } - if (hasLocalBin(projectRoot, "biome")) { + if (localBin(projectRoot, "biome")) { return [join(projectRoot, "node_modules", ".bin", "biome"), "check", "--no-colors", absPath]; } return null; @@ -78,17 +92,18 @@ function commandFor(absPath: string, projectRoot: string): string[] | null { return null; } if (ext === ".php") { - // Prefer static analysis; fall back to Pint style check, then php -l syntax. + // Static analysis / style only — no php -l fallback (syntax-only is a different semantic level). if (onPath("phpstan")) return ["phpstan", "analyse", "--no-progress", "--error-format=raw", absPath]; if (onPath("pint")) return ["pint", "--test", absPath]; - if (onPath("php")) return ["php", "-l", absPath]; return null; } if (ext === ".kt" || ext === ".kts") { - if (onPath("ktlint")) return ["ktlint", "--reporter=plain", absPath]; + // No --reporter flag: ktlint 1.x changed reporter CLI; default plain output is fine. + if (onPath("ktlint")) return ["ktlint", absPath]; return null; } - if (ext === ".sh" || ext === ".bash" || ext === ".zsh") { + if (ext === ".sh" || ext === ".bash") { + // shellcheck does not support zsh — do not include .zsh (false positives). if (onPath("shellcheck")) return ["shellcheck", "-f", "gcc", absPath]; return null; } @@ -105,7 +120,7 @@ function commandFor(absPath: string, projectRoot: string): string[] | null { */ export function runDiagnostics(absPath: string, projectRoot: string): string | null { if (!cfg.enabled) return null; - const cmd = commandFor(absPath, projectRoot); + const cmd = resolveDiagnosticsCommand(absPath, projectRoot); if (!cmd) return null; let out: string;