diff --git a/.github/workflows/check-opencode-annotations.yml b/.github/workflows/check-opencode-annotations.yml index 885fa7e458e..810371d6d3e 100644 --- a/.github/workflows/check-opencode-annotations.yml +++ b/.github/workflows/check-opencode-annotations.yml @@ -37,3 +37,8 @@ jobs: else echo "No PR base SHA available (workflow_dispatch without PR context) — skipping." fi + + # kilocode_change start + - name: Check workflow allowlist + run: bun run script/check-workflows.ts + # kilocode_change end diff --git a/AGENTS.md b/AGENTS.md index 7285909eb25..cc5eda0d054 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,7 @@ Kilo CLI is an open source AI coding agent that generates code from natural lang - **Source links**: After adding or changing URLs in `packages/kilo-vscode/`, `packages/kilo-vscode/webview-ui/`, or `packages/opencode/src/`, run `bun run script/extract-source-links.ts` from the repo root and commit the updated `packages/kilo-docs/source-links.md`. CI runs this check — the build fails if the file is stale. - **kilocode_change check**: `bun run check-kilocode-change` from `packages/kilo-vscode/`. CI runs this — `kilocode_change` is a marker for upstream merge conflicts and must not appear in `packages/kilo-vscode/` or `packages/kilo-ui/` (these are entirely Kilo Code additions). Remove the markers before pushing. - **opencode annotation check**: `bun run script/check-opencode-annotations.ts` from repo root. CI runs this on PRs touching `packages/opencode/` — every Kilo-specific change in shared opencode files must be annotated with `kilocode_change` markers. Exempt paths (no markers needed): `packages/opencode/src/kilocode/`, `packages/opencode/test/kilocode/`, and any path containing `kilocode` in the name. +- **workflow allowlist**: `bun run script/check-workflows.ts` from repo root. CI runs this as part of the annotations workflow — any `.yml` / `.yaml` file added to or removed from `.github/workflows/` must be reflected in the hardcoded list in `script/check-workflows.ts`. Prevents upstream-merged workflows from silently starting to run in our CI. - **Backend/SDK programmatic testing**: see [TESTING.md](./TESTING.md) for spawning the local main-branch backend (`bun dev serve`) and driving it via `curl` — use this instead of `kilo serve` (prod binary) when testing backend fixes. ## Quality Checks diff --git a/bun.lock b/bun.lock index 5ea5e1bf75e..3ff080a4b21 100644 --- a/bun.lock +++ b/bun.lock @@ -358,6 +358,7 @@ "ai-gateway-provider": "3.1.2", "bonjour-service": "1.3.0", "bun-pty": "0.4.8", + "chardet": "2.1.1", "chokidar": "4.0.3", "cli-sound": "1.1.3", "clipboardy": "4.0.0", @@ -376,7 +377,6 @@ "iconv-lite": "0.7.2", "ignore": "7.0.5", "immer": "11.1.4", - "jschardet": "3.1.4", "jsonc-parser": "3.3.1", "mime-types": "3.0.2", "minimatch": "10.2.5", @@ -3300,8 +3300,6 @@ "jsbi": ["jsbi@4.3.2", "", {}, "sha512-9fqMSQbhJykSeii05nxKl4m6Eqn2P6rOlYiS+C5Dr/HPIU/7yZxu5qzbs40tgaFORiw2Amd0mirjxatXYMkIew=="], - "jschardet": ["jschardet@3.1.4", "", {}, "sha512-/kmVISmrwVwtyYU40iQUOp3SUPk2dhNCMsZBQX0R1/jZ8maaXJ/oZIzUOiyOqcgtLnETFKYChbJ5iDC/eWmFHg=="], - "jsesc": ["jsesc@3.1.0", "", { "bin": { "jsesc": "bin/jsesc" } }, "sha512-/sM3dO2FOzXjKQhJuo0Q173wf2KOo8t4I8vHy6lF9poUp7bKT0/NHE8fPX23PwfhnykfqnC2xRxOnVw5XuGIaA=="], "json-bigint": ["json-bigint@1.0.0", "", { "dependencies": { "bignumber.js": "^9.0.0" } }, "sha512-SiPv/8VpZuWbvLSMtTDU8hEfrZWg/mH/nV/b4o0CYbSxu1UIQPLdwKOCIyLQX+VIPO5vrLX3i8qtqFyhdPSUSQ=="], diff --git a/nix/hashes.json b/nix/hashes.json index e6ab26053e9..04605df30f9 100644 --- a/nix/hashes.json +++ b/nix/hashes.json @@ -1,8 +1,8 @@ { "nodeModules": { - "x86_64-linux": "sha256-1SpPwQiU4XdqJNJhw9Abf2IiBMxv22waspgDTylL2yg=", - "aarch64-linux": "sha256-g+wFoSCCfz0lZym83gHvAPj1R9vIpF4/7jK3bUGCMP4=", - "aarch64-darwin": "sha256-aGNmAN6RWL+4V6kxckMrOPJIgvhnomwILcNBI27VapA=", - "x86_64-darwin": "sha256-KXPaxTzaypPSjBvmUJVICAcjg6XVnG4EKCRFCcUZ3oI=" + "x86_64-linux": "sha256-BkFDOCEvivFrxKOvMjAIhj3QZwmb9vE/KvCZv5puw6E=", + "aarch64-linux": "sha256-sTkfQNjqCNCxmHbpQ8woA6azoiyD37e8Xy7UwB6eNsA=", + "aarch64-darwin": "sha256-lKuagsTHqij5C764DM3d+BOe4JKs2KV8XiwSlJItGMg=", + "x86_64-darwin": "sha256-g5gZMHLq8feSZHVaCOLGU6Tj2Qw56Zogif3pT9EIlz8=" } } diff --git a/packages/kilo-docs/pages/code-with-ai/features/file-encoding.md b/packages/kilo-docs/pages/code-with-ai/features/file-encoding.md index 416d8cac39a..746d12dc738 100644 --- a/packages/kilo-docs/pages/code-with-ai/features/file-encoding.md +++ b/packages/kilo-docs/pages/code-with-ai/features/file-encoding.md @@ -15,7 +15,7 @@ Kilo automatically detects the text encoding of each file it reads and preserves - Shift_JIS, EUC-JP, GB2312, Big5, EUC-KR - Windows-1251, KOI8-R - The ISO-8859 family -- Other common legacy Latin and CJK encodings detected by [jschardet](https://github.com/aadsm/jschardet) and decoded by [iconv-lite](https://github.com/ashtuchkin/iconv-lite) +- Other common legacy Latin and CJK encodings detected by [chardet](https://github.com/runk/node-chardet) and decoded by [iconv-lite](https://github.com/ashtuchkin/iconv-lite) New files Kilo creates are always UTF-8 without a BOM. Encoding detection only runs when Kilo reads or overwrites an existing file. diff --git a/packages/opencode/package.json b/packages/opencode/package.json index 42d0e348285..c7651a6d8fa 100644 --- a/packages/opencode/package.json +++ b/packages/opencode/package.json @@ -142,6 +142,7 @@ "ai-gateway-provider": "3.1.2", "bonjour-service": "1.3.0", "bun-pty": "0.4.8", + "chardet": "2.1.1", "chokidar": "4.0.3", "cli-sound": "1.1.3", "clipboardy": "4.0.0", @@ -160,7 +161,6 @@ "iconv-lite": "0.7.2", "ignore": "7.0.5", "immer": "11.1.4", - "jschardet": "3.1.4", "jsonc-parser": "3.3.1", "mime-types": "3.0.2", "minimatch": "10.2.5", diff --git a/packages/opencode/src/jschardet.d.ts b/packages/opencode/src/jschardet.d.ts deleted file mode 100644 index 7e819af9cc1..00000000000 --- a/packages/opencode/src/jschardet.d.ts +++ /dev/null @@ -1,14 +0,0 @@ -declare module "jschardet" { - export interface Result { - encoding?: string - confidence?: number - } - - export function detect(input: ArrayLike): Result - - const api: { - detect(input: ArrayLike): Result - } - - export default api -} diff --git a/packages/opencode/src/kilocode/encoding.ts b/packages/opencode/src/kilocode/encoding.ts index 314384fa079..628c113fb15 100644 --- a/packages/opencode/src/kilocode/encoding.ts +++ b/packages/opencode/src/kilocode/encoding.ts @@ -1,7 +1,7 @@ import { readFile, writeFile, mkdir } from "fs/promises" import { readFileSync } from "fs" import { dirname } from "path" -import jschardet from "jschardet" +import chardet from "chardet" import iconv from "iconv-lite" /** @@ -9,9 +9,9 @@ import iconv from "iconv-lite" * * Supported: * - UTF-8 (with or without BOM) - * - UTF-16 LE/BE with BOM (detected by jschardet) - * - UTF-32 LE/BE with BOM (detected by jschardet) - * - Legacy Latin and CJK encodings (detected by jschardet) + * - UTF-16 LE/BE with BOM (detected by chardet) + * - UTF-32 LE/BE with BOM (detected by chardet) + * - Legacy Latin and CJK encodings (detected by chardet) * * Not supported: * - UTF-16 or UTF-32 without BOM (ambiguous, rare) @@ -19,7 +19,7 @@ import iconv from "iconv-lite" * Detection strategy: * 1. If the bytes are valid UTF-8, treat as UTF-8 (tracking the presence of a * BOM so it can be written back). - * 2. Otherwise, trust jschardet. jschardet only reports the wide UTF variants + * 2. Otherwise, trust chardet. chardet only reports the wide UTF variants * when a BOM is present, which aligns with the contract above. * * iconv-lite's UTF codecs strip BOMs on decode and do not emit them on encode, @@ -33,29 +33,47 @@ export namespace Encoding { * track the "with BOM" case explicitly to round-trip it faithfully. */ export const UTF8_BOM = "utf-8-bom" - const UTF8_BOM_BYTES = Buffer.from([0xef, 0xbb, 0xbf]) + const BOMS = { + "utf-8-bom": Buffer.from([0xef, 0xbb, 0xbf]), + "utf-16le": Buffer.from([0xff, 0xfe]), + "utf-16be": Buffer.from([0xfe, 0xff]), + "utf-32le": Buffer.from([0xff, 0xfe, 0x00, 0x00]), + "utf-32be": Buffer.from([0x00, 0x00, 0xfe, 0xff]), + } + + function startsWith(bytes: Buffer, bom: Buffer, limit: number): boolean { + return limit >= bom.length && bytes.subarray(0, bom.length).equals(bom) + } function hasUtf8Bom(bytes: Buffer): boolean { - return bytes.length >= 3 && bytes[0] === 0xef && bytes[1] === 0xbb && bytes[2] === 0xbf + return startsWith(bytes, BOMS["utf-8-bom"], bytes.length) } /** True if `bytes[0..limit]` starts with a UTF-16 LE or BE byte-order mark. */ export function hasUtf16Bom(bytes: Buffer, limit = bytes.length): boolean { - if (limit < 2) return false - // UTF-32 LE starts with FF FE 00 00, so exclude that to avoid misclassifying it as UTF-16 LE. + // UTF-32 LE starts with FF FE 00 00, so exclude it to avoid misclassifying as UTF-16 LE. if (hasUtf32Bom(bytes, limit)) return false - return (bytes[0] === 0xff && bytes[1] === 0xfe) || (bytes[0] === 0xfe && bytes[1] === 0xff) + return startsWith(bytes, BOMS["utf-16le"], limit) || startsWith(bytes, BOMS["utf-16be"], limit) } /** True if `bytes[0..limit]` starts with a UTF-32 LE or BE byte-order mark. */ export function hasUtf32Bom(bytes: Buffer, limit = bytes.length): boolean { - if (limit < 4) return false - const le = bytes[0] === 0xff && bytes[1] === 0xfe && bytes[2] === 0x00 && bytes[3] === 0x00 - const be = bytes[0] === 0x00 && bytes[1] === 0x00 && bytes[2] === 0xfe && bytes[3] === 0xff - return le || be + return startsWith(bytes, BOMS["utf-32le"], limit) || startsWith(bytes, BOMS["utf-32be"], limit) } - /** Remap jschardet labels to iconv-lite compatible names. */ + /** + * Canonicalize chardet labels to a stable lowercase-hyphenated form. + * + * iconv-lite already accepts every label chardet emits (e.g. "UTF-16 LE", + * "Shift_JIS", "KOI8-R"), so this map is not required to make decode/encode + * work. Its job is to give the rest of the codebase a consistent label — + * callers compare against `"utf-16le"`, `"windows-1251"`, etc., and should + * not have to account for chardet's casing or whitespace conventions. + * + * (ISO-2022-* is the one family iconv-lite does not support under any + * alias; those labels fall through to the `encodingExists` guard in + * `detect()` and are rejected to UTF-8.) + */ function normalize(name: string): string { const lower = name.toLowerCase().replace(/[^a-z0-9]/g, "") const map: Record = { @@ -64,7 +82,6 @@ export namespace Encoding { utf16be: "utf-16be", utf32le: "utf-32le", utf32be: "utf-32be", - ascii: "utf-8", iso88591: "iso-8859-1", iso88592: "iso-8859-2", iso88595: "iso-8859-5", @@ -82,13 +99,8 @@ export namespace Encoding { euckr: "euc-kr", iso2022kr: "iso-2022-kr", big5: "big5", - gb2312: "gb2312", gb18030: "gb18030", koi8r: "koi8-r", - maccyrillic: "x-mac-cyrillic", - ibm855: "cp855", - ibm866: "cp866", - tis620: "tis-620", } return map[lower] ?? name } @@ -105,9 +117,9 @@ export namespace Encoding { export function detect(bytes: Buffer): string { if (bytes.length === 0) return DEFAULT if (isUtf8(bytes)) return hasUtf8Bom(bytes) ? UTF8_BOM : DEFAULT - const result = jschardet.detect(bytes) - if (!result.encoding) return DEFAULT - const enc = normalize(result.encoding) + const result = chardet.detect(bytes) + if (!result) return DEFAULT + const enc = normalize(result) if (!iconv.encodingExists(enc)) return DEFAULT return enc } @@ -124,14 +136,9 @@ export namespace Encoding { // first so we never emit a double BOM when the decoded text already // contains one (e.g. if a tool round-trips content verbatim). const body = text.charCodeAt(0) === 0xfeff ? text.slice(1) : text - if (encoding === UTF8_BOM) return Buffer.concat([UTF8_BOM_BYTES, iconv.encode(body, "utf-8")]) - const lower = encoding.toLowerCase() - if (lower === "utf-16le") return Buffer.concat([Buffer.from([0xff, 0xfe]), iconv.encode(body, encoding)]) - if (lower === "utf-16be") return Buffer.concat([Buffer.from([0xfe, 0xff]), iconv.encode(body, encoding)]) - if (lower === "utf-32le") - return Buffer.concat([Buffer.from([0xff, 0xfe, 0x00, 0x00]), iconv.encode(body, encoding)]) - if (lower === "utf-32be") - return Buffer.concat([Buffer.from([0x00, 0x00, 0xfe, 0xff]), iconv.encode(body, encoding)]) + const key = encoding === UTF8_BOM ? UTF8_BOM : encoding.toLowerCase() + const bom = BOMS[key as keyof typeof BOMS] + if (bom) return Buffer.concat([bom, iconv.encode(body, key === UTF8_BOM ? "utf-8" : key)]) return iconv.encode(text, encoding) } diff --git a/packages/opencode/test/kilocode/encoding.test.ts b/packages/opencode/test/kilocode/encoding.test.ts index 177600f0cfb..009e839eef0 100644 --- a/packages/opencode/test/kilocode/encoding.test.ts +++ b/packages/opencode/test/kilocode/encoding.test.ts @@ -3,7 +3,7 @@ // by exercising detect/decode/encode/read/write/readSync directly, without // going through the Effect runtime, agent harness, or tool pipeline. They are // cheap, fast, and cover the internal branches (BOM handling, ASCII/UTF-8 -// normalization, jschardet fallback, unsupported encoding rejection) that the +// normalization, chardet fallback, unsupported encoding rejection) that the // integration tests cannot hit deterministically. import { describe, expect, test } from "bun:test" @@ -35,10 +35,10 @@ describe("Encoding.detect", () => { expect(Encoding.detect(Buffer.alloc(0))).toBe(Encoding.DEFAULT) }) - test("plain ASCII is normalized to utf-8 (not 'ascii')", () => { - // jschardet reports "ascii" for pure-ASCII input; the namespace treats - // that as UTF-8 because UTF-8 is an ASCII superset and iconv-lite doesn't - // expose an "ascii" label that round-trips identically. + test("plain ASCII is reported as utf-8", () => { + // Plain ASCII is valid UTF-8, so the detector short-circuits on the + // isUtf8 check and never reaches chardet. UTF-8 is an ASCII superset, + // so this label round-trips identically through iconv-lite. expect(Encoding.detect(Buffer.from("plain ascii text\n"))).toBe("utf-8") }) @@ -52,8 +52,8 @@ describe("Encoding.detect", () => { }) test("BOM-less UTF-8 containing multi-byte chars is not misdetected", () => { - // Regression guard: bytes that are valid UTF-8 must skip the jschardet - // branch. jschardet has been known to misfire on short CJK samples. + // Regression guard: bytes that are valid UTF-8 must skip the chardet + // branch. Encoding detectors have been known to misfire on short CJK samples. expect(Encoding.detect(Buffer.from("한글 テスト 中文", "utf-8"))).toBe("utf-8") }) @@ -231,13 +231,23 @@ describe("Encoding.hasUtf32Bom", () => { }) describe("Encoding.read / Encoding.readSync / Encoding.write", () => { + // chardet is noticeably more conservative than other detectors (jschardet, + // ICU) on tiny samples: a 12-byte Shift_JIS phrase collides with the + // windows-1252 profile and is misclassified. In practice this is fine, + // because the tool pipeline only runs detection on files the agent is about + // to read or patch — real source files and documents carry far more than 12 + // bytes of characteristic content, which is plenty for chardet to lock + // onto the right encoding. The short-sample cliff only matters for + // synthetic fixtures like this one, so we pad the sample to the same body + // of Japanese text the rest of the suite already relies on. + const shiftJisSample = "こんにちは、世界！日本語のテストです。" + test("read detects and decodes Shift_JIS asynchronously", async () => { await tmp(async (dir) => { const filepath = path.join(dir, "sj.txt") - const text = "日本語テスト" - await fs.writeFile(filepath, iconv.encode(text, "Shift_JIS")) + await fs.writeFile(filepath, iconv.encode(shiftJisSample, "Shift_JIS")) const result = await Encoding.read(filepath) - expect(result.text).toBe(text) + expect(result.text).toBe(shiftJisSample) expect(result.encoding.toLowerCase()).toBe("shift_jis") }) }) @@ -245,8 +255,7 @@ describe("Encoding.read / Encoding.readSync / Encoding.write", () => { test("readSync mirrors read for the same input", async () => { await tmp(async (dir) => { const filepath = path.join(dir, "sj.txt") - const text = "日本語テスト" - await fs.writeFile(filepath, iconv.encode(text, "Shift_JIS")) + await fs.writeFile(filepath, iconv.encode(shiftJisSample, "Shift_JIS")) const sync = Encoding.readSync(filepath) const async_ = await Encoding.read(filepath) expect(sync).toEqual(async_) diff --git a/packages/opencode/test/kilocode/patch.test.ts b/packages/opencode/test/kilocode/patch.test.ts index 8dc13ab8713..86ef2195467 100644 --- a/packages/opencode/test/kilocode/patch.test.ts +++ b/packages/opencode/test/kilocode/patch.test.ts @@ -107,7 +107,7 @@ describe("Patch encoding preservation", () => { test("preserves Shift_JIS encoding through update", async () => { const file = path.join(dir, "jp.txt") - // jschardet needs enough characteristic bytes to identify Shift_JIS. A + // chardet needs enough characteristic bytes to identify Shift_JIS. A // single 19-byte phrase looks like windows-1252, so the sample is padded // to match the body of Japanese text the tool tests already rely on. const sample = "こんにちは、世界！日本語のテストです。" diff --git a/packages/opencode/test/kilocode/tool-encoding.test.ts b/packages/opencode/test/kilocode/tool-encoding.test.ts index a200670d48b..42f21e95ee9 100644 --- a/packages/opencode/test/kilocode/tool-encoding.test.ts +++ b/packages/opencode/test/kilocode/tool-encoding.test.ts @@ -372,7 +372,7 @@ describe("tool encoding preservation", () => { provideTmpdirInstance((dir) => Effect.gen(function* () { const filepath = path.join(dir, "doc.txt") - // Pad with additional Shift_JIS text so jschardet has enough bytes + // Pad with additional Shift_JIS text so chardet has enough bytes // to confidently identify the encoding. const pad = samples.shiftJis + "\n" const original = pad + "日本語\n日本語\n日本語\n" + pad diff --git a/script/check-workflows.ts b/script/check-workflows.ts new file mode 100644 index 00000000000..a7536cd0799 --- /dev/null +++ b/script/check-workflows.ts @@ -0,0 +1,81 @@ +#!/usr/bin/env bun +// kilocode_change - new file + +/** + * Guards against accidentally inheriting workflows from upstream opencode. + * + * We regularly merge upstream. When upstream adds a new workflow under + * `.github/workflows/`, it silently starts running in our CI unless we + * explicitly review and accept it. This check makes that decision explicit: + * the list of allowed workflows is hardcoded below, and any drift (added or + * removed file in `.github/workflows/`) fails CI until the list is updated + * deliberately. + * + * Only runnable workflows are checked (`.yml` / `.yaml`). Files under + * `.github/workflows/disabled/` are Kilo-specific and can't run, so they're + * not tracked here. + * + * To accept a new workflow: add its filename to `active`. + * To drop one: remove its filename from the list. + */ + +import { readdirSync } from "node:fs" +import path from "node:path" + +const ROOT = path.resolve(import.meta.dir, "..") +const DIR = path.join(ROOT, ".github", "workflows") + +// Workflows we have deliberately accepted into CI. Sort alphabetically. +const active = new Set([ + "auto-docs.yml", + "beta.yml", + "check-md-table-padding.yml", + "check-opencode-annotations.yml", + "check-org-member.yml", + "close-issues.yml", + "close-stale-prs.yml", + "containers.yml", + "docs-build.yml", + "docs-check-links.yml", + "duplicate-issues.yml", + "generate.yml", + "nix-eval.yml", + "nix-hashes.yml", + "publish.yml", + "smoke-test.yml", + "source-check-links.yml", + "test-vscode.yml", + "test.yml", + "triage.yml", + "typecheck.yml", + "visual-regression.yml", + "watch-opencode-releases.yml", +]) + +// GitHub picks up both .yml and .yaml in .github/workflows/. We accept both so +// an upstream `.yaml` addition also shows up as unexpected drift. +const isWorkflow = (f: string) => f.endsWith(".yml") || f.endsWith(".yaml") +const actualActive = new Set(readdirSync(DIR).filter(isWorkflow)) + +const missing = [...active].filter((f) => !actualActive.has(f)).sort() +const extra = [...actualActive].filter((f) => !active.has(f)).sort() +const errs: string[] = [] +for (const f of extra) { + errs.push(`unexpected workflow: ${f} — if this was added intentionally, add it to script/check-workflows.ts`) +} +for (const f of missing) { + errs.push( + `expected workflow not found: ${f} — if this was removed intentionally, remove it from script/check-workflows.ts`, + ) +} + +if (errs.length === 0) { + console.log(`check-workflows: ok (${actualActive.size} workflows).`) + process.exit(0) +} + +for (const e of errs) console.error(e) +console.error("") +console.error(`Found ${errs.length} workflow drift issue(s).`) +console.error("This guard prevents upstream-merged workflows from silently running in our CI.") +process.exit(1)