diff --git a/.changeset/headless-run-honest-exit.md b/.changeset/headless-run-honest-exit.md new file mode 100644 index 000000000000..37d00a84931f --- /dev/null +++ b/.changeset/headless-run-honest-exit.md @@ -0,0 +1,13 @@ +--- +"@kilocode/cli": patch +--- + +Non-interactive `kilo run` no longer reports success for runs that did not complete. A plain +headless run (neither `--auto` nor `--dangerously-skip-permissions`) in which the CLI +auto-rejected at least one permission ask now exits 1 with a stderr diagnostic naming the cause, +and a run whose session errors mid-stream now prints that diagnostic to stderr under +`--format json` as well (previously the JSON branch swallowed it). Runs that complete their turn +with no auto-rejected permission still exit 0. Under `--format json` the auto-reject path adds a +new `error` event to the stream; existing event shapes are unchanged. The same exit-1 rule applies +to a plain non-interactive `--attach` run that auto-rejects an ask (that run was equally crippled); +interactive mode is untouched. diff --git a/.github/docs-sync/edit.mjs b/.github/docs-sync/edit.mjs index 777c3a2929ca..167656eda350 100644 --- a/.github/docs-sync/edit.mjs +++ b/.github/docs-sync/edit.mjs @@ -82,8 +82,13 @@ Batch specifics for this run: the PRs to handle are in the attached ${batchFile} break } + // Headless `kilo run` auto-rejects every permission ask; without --auto the + // agent cannot run shell commands. SECURITY: --auto grants unrestricted bash + // to an agent steered by external PR content. Hardening deferred: a scoped + // permission.bash map via KILO_CONFIG_CONTENT should replace --auto once the + // required shell patterns are stable (see PR #12605 review thread). const result = runKilo({ - args: ["run", prompt, "-m", model, "--variant", "high", "--dir", process.cwd(), "-f", batchFile, "-f", triageFile], + args: ["run", "--auto", prompt, "-m", model, "--variant", "high", "--dir", process.cwd(), "-f", batchFile, "-f", triageFile], timeoutMs: Math.min(BATCH_TIMEOUT_MS, left), streamStdout: true, label: `edit batch ${index} attempt ${attempt}`, diff --git a/.github/docs-sync/lib.mjs b/.github/docs-sync/lib.mjs index 67c13354d88c..6aaf9dc6c36d 100644 --- a/.github/docs-sync/lib.mjs +++ b/.github/docs-sync/lib.mjs @@ -167,13 +167,53 @@ export function sleepSync(ms) { const STDERR_TAIL_LINES = 20 const STDERR_TAIL_CHARS = 4_000 +// CSI sequences (colour, cursor moves, erases). kilo renders its TUI to stderr, +// so an unstripped tail lands in the rolling PR's pending table as +// "^[[0m→ ^[[0mRead packages/..." and the cause is unreadable. Stripped before +// the line/char slice so escapes do not eat the budget. The persisted +// docs-sync-out/kilo-stderr-*.log stays raw — that is the debugging record. +// eslint-disable-next-line no-control-regex +const ANSI_CSI = /\u001b\[[0-9;?]*[ -/]*[@-~]/g + function tailText(text, { lines = STDERR_TAIL_LINES, chars = STDERR_TAIL_CHARS } = {}) { - const s = String(text ?? "").trim() + const s = String(text ?? "") + .replace(ANSI_CSI, "") + .trim() if (!s) return "" const lastLines = s.split("\n").slice(-lines).join("\n") return lastLines.length > chars ? lastLines.slice(-chars) : lastLines } +/** + * Artifact files are raw: GitHub masks secret values in log streams only, and the runner + * env holds long-lived secrets (KILO_API_KEY), so exact values of secret-looking env vars + * are redacted before stdout/stderr is persisted or printed. + * Matching is exact-substring and case-sensitive on values — JSON-escaped, base64'd, or + * line-wrapped renderings and values shorter than 8 chars survive (same limitation as + * GitHub's own log masking); this is defense-in-depth, not a guarantee the logs are clean. + */ +export function redactEnvSecrets(text) { + let out = String(text ?? "") + // Also match CREDENTIAL/PASSWORD/ORG_ID/_PAT (e.g. KILO_ORG_ID, GH_PAT) beyond KEY|TOKEN|SECRET. + const nameRe = /KEY|TOKEN|SECRET|CREDENTIAL|PASSWORD|ORG_ID|_PAT$/i + const candidates = [] + for (const [name, value] of Object.entries(process.env)) { + if (!nameRe.test(name)) continue + if (typeof value !== "string" || value.length < 8) continue + candidates.push(value) + } + // Longer values first so a shorter secret that is a prefix of a longer one cannot leave a remainder. + candidates.sort((a, b) => b.length - a.length) + for (const value of candidates) { + if (!out.includes(value)) continue + out = out.split(value).join("***") + } + return out +} + +/** Max bytes of child stderr persisted to docs-sync-out/ (full buffer, not the console tail). */ +const STDERR_LOG_MAX_CHARS = 8 * 1024 * 1024 + /** * Run `kilo` via spawnSync so stderr is always recoverable — including when * the child exits 0 after writing a diagnostic (execFileSync cannot return @@ -181,6 +221,10 @@ function tailText(text, { lines = STDERR_TAIL_LINES, chars = STDERR_TAIL_CHARS } * * streamStdout:true → inherit fd 1 (edit live log); false → capture stdout * (triage parses it). stderr is always buffered. + * + * Always writes the full captured stderr to + * docs-sync-out/kilo-stderr-.log (unconditional — success and + * failure). The console return value still uses the short tailText. */ export function runKilo({ args, timeoutMs, streamStdout = false, label = "kilo" }) { const result = spawnSync("kilo", args, { @@ -193,16 +237,30 @@ export function runKilo({ args, timeoutMs, streamStdout = false, label = "kilo" const timedOut = Boolean(result.error && result.error.code === "ETIMEDOUT") const exitCode = typeof result.status === "number" ? result.status : timedOut ? null : result.status === null ? null : result.status - const stderrTail = tailText(result.stderr) - const stdout = streamStdout ? "" : String(result.stdout ?? "") + const stderrRaw = String(result.stderr ?? "") + const stderrSafe = redactEnvSecrets(stderrRaw) + const stderrTail = tailText(stderrSafe) + const stdoutSafe = streamStdout ? "" : redactEnvSecrets(String(result.stdout ?? "")) // ok is "process finished without OS-level failure". Callers still treat a // missing summary / unparseable output as failure even when ok is true — // exit 0 is not success for the docs-sync bot. const ok = !result.error && result.status === 0 + // Persist full stderr on every call (not gated on ok/exitCode/summary). Cap is + // generous (megabytes) so long batch dumps keep auto-rejecting lines; console + // still uses the short tail above. + try { + fs.mkdirSync("docs-sync-out", { recursive: true }) + const safe = label.replace(/[^A-Za-z0-9._-]/g, "-") + const body = stderrSafe.length > STDERR_LOG_MAX_CHARS ? stderrSafe.slice(-STDERR_LOG_MAX_CHARS) : stderrSafe + fs.writeFileSync(`docs-sync-out/kilo-stderr-${safe}.log`, body) + } catch (err) { + console.warn(`${label}: failed to write kilo-stderr log: ${err.message}`) + } + if (result.error && !timedOut) { console.warn(`${label}: spawn error: ${result.error.message}`) } - return { ok, stdout, stderrTail, exitCode, timedOut } + return { ok, stdout: stdoutSafe, stderrTail, exitCode, timedOut } } diff --git a/.github/docs-sync/redact-stream.mjs b/.github/docs-sync/redact-stream.mjs new file mode 100644 index 000000000000..341aea34ad77 --- /dev/null +++ b/.github/docs-sync/redact-stream.mjs @@ -0,0 +1,25 @@ +// kilocode_change - new file + +/** + * Line-wise stdin→stdout filter that redacts secret-looking env values. + * Used in the docs-sync workflow so kilo stdout piped to edit-log.txt is safe. + * Env values contain no newlines, so line-wise processing never splits a value. + */ + +import { redactEnvSecrets } from "./lib.mjs" + +let carry = "" + +process.stdin.setEncoding("utf8") +process.stdin.on("data", (chunk) => { + carry += chunk + let idx + while ((idx = carry.indexOf("\n")) !== -1) { + const line = carry.slice(0, idx + 1) + carry = carry.slice(idx + 1) + process.stdout.write(redactEnvSecrets(line)) + } +}) +process.stdin.on("end", () => { + if (carry.length > 0) process.stdout.write(redactEnvSecrets(carry)) +}) diff --git a/.github/docs-sync/selftest.mjs b/.github/docs-sync/selftest.mjs index fe2cb1faec34..b720ba67cb5a 100644 --- a/.github/docs-sync/selftest.mjs +++ b/.github/docs-sync/selftest.mjs @@ -55,7 +55,7 @@ function writeExecutable(filePath, body) { function makeStubKiloDir({ mode, callLog, stderrText = "event stream disconnected" }) { const dir = mktemp("docs-sync-kilo-") const kiloPath = path.join(dir, "kilo") - // mode: "stderr-exit0" | "record" | "partial-triage" | "mixed-triage" + // mode: "stderr-exit0" | "record" | "partial-triage" | "mixed-triage" | "write-edit-summary" const script = `#!/usr/bin/env node const fs = require("node:fs"); const path = require("node:path"); @@ -81,6 +81,22 @@ let chunk = []; if (fileArg && fs.existsSync(fileArg)) { try { chunk = JSON.parse(fs.readFileSync(fileArg, "utf8")); } catch { chunk = []; } } +if (mode === "write-edit-summary") { + // Success path: write the batch summary so edit.mjs returns true, while still + // emitting stderr so selftest can assert runKilo persisted it unconditionally. + process.stderr.write(stderrText + "\\n"); + const m = fileArg && String(fileArg).match(/edit-batch-(\\d+)\\.json/); + const index = m ? m[1] : "0"; + const summary = chunk.map((d) => ({ + pr: d.number, + url: d.url, + action: "skipped", + reason: "selftest stub", + })); + fs.mkdirSync("docs-sync-out", { recursive: true }); + fs.writeFileSync("docs-sync-out/edit-summary-" + index + ".json", JSON.stringify(summary)); + process.exit(0); +} if (mode === "partial-triage") { // Classify only a proper subset (first URL) of the chunk. const owned = chunk.slice(0, Math.max(0, chunk.length - 1)); @@ -116,6 +132,21 @@ if (mode === "mixed-triage") { process.stdout.write(JSON.stringify(entries) + "\\n"); process.exit(0); } +if (mode === "triage-embed-env-secret") { + // Valid triage JSON with a secret env value embedded in a string field + // (stdout is persisted to triage-raw-*.txt; must be redacted at capture). + const secret = process.env.KILO_API_KEY || "missing-secret"; + const entries = chunk.map((d) => ({ + pr: d.number, + url: d.url, + docs_worthy: true, + reason: "needs docs; diagnostic=" + secret, + target_sections: ["overview"], + priority: "high", + })); + process.stdout.write(JSON.stringify(entries) + "\\n"); + process.exit(0); +} process.stderr.write("unknown stub mode\\n"); process.exit(1); ` @@ -129,7 +160,9 @@ function gitIn(cwd, args, env = {}) { env: { ...process.env, ...env }, stdio: ["ignore", "pipe", "pipe"], encoding: "utf8", - }).toString().trim() + }) + .toString() + .trim() } function makeGitRunner(cwd, env = {}) { @@ -247,14 +280,19 @@ function case1_mergeOrFallback() { env: { ...process.env, ...env }, stdio: ["ignore", "pipe", "pipe"], encoding: "utf8", - }).toString().trim() + }) + .toString() + .trim() - assert.throws(() => mergeOrFallback({ branch: DEFAULT_BRANCH, git }), (err) => { - // Must throw the original merge error, not a merge --abort failure - const msg = String(err?.stderr ?? err?.message ?? err) - assert.ok(!/no merge to abort/i.test(msg), `should not reach merge --abort: ${msg}`) - return true - }) + assert.throws( + () => mergeOrFallback({ branch: DEFAULT_BRANCH, git }), + (err) => { + // Must throw the original merge error, not a merge --abort failure + const msg = String(err?.stderr ?? err?.message ?? err) + assert.ok(!/no merge to abort/i.test(msg), `should not reach merge --abort: ${msg}`) + return true + }, + ) } } @@ -372,6 +410,450 @@ function case2_defectB() { } } +// --------------------------------------------------------------------------- +// Case 2b — AC4a: every docs-sync kilo run argv carries --auto +// --------------------------------------------------------------------------- +/** Slice `args: [` … matching `]` from source (newlines allowed inside). */ +function extractArgsArraySlice(source) { + const start = source.indexOf("args: [") + assert.ok(start >= 0, "args: [ not found in source") + let i = start + "args: ".length + assert.equal(source[i], "[") + let depth = 0 + for (; i < source.length; i++) { + const ch = source[i] + if (ch === "[") depth++ + else if (ch === "]") { + depth-- + if (depth === 0) return source.slice(start, i + 1) + } + } + throw new assert.AssertionError({ message: "unclosed args: [ array in source" }) +} + +/** Label → kilo-stderr filename rule (must match lib.mjs runKilo). */ +function kiloStderrLogName(label) { + return `kilo-stderr-${String(label).replace(/[^A-Za-z0-9._-]/g, "-")}.log` +} + +function case2b_autoFlag() { + console.log("case 2b: AC4a (--auto on every docs-sync kilo run)") + + // (i) region-scoped static check on triage.mjs / edit.mjs argv arrays + for (const name of ["triage.mjs", "edit.mjs"]) { + const src = fs.readFileSync(path.join(HERE, name), "utf8") + const slice = extractArgsArraySlice(src) + assert.ok(slice.includes('"--auto"'), `${name} args array must contain "--auto"; got:\n${slice}`) + } + + // (ii) Fix verify failures step: join the run: | block and require --auto on kilo run + { + const yml = fs.readFileSync(path.join(HERE, "..", "workflows", "docs-sync.yml"), "utf8") + const stepIdx = yml.indexOf("Fix verify failures") + assert.ok(stepIdx >= 0, "Fix verify failures step missing") + const afterStep = yml.slice(stepIdx) + const runIdx = afterStep.indexOf("run: |") + assert.ok(runIdx >= 0, "run: | missing after Fix verify failures") + const blockStart = stepIdx + runIdx + "run: |".length + const rest = yml.slice(blockStart) + // Block ends at next unindented step key or EOF — collect indented lines + const lines = [] + for (const line of rest.split("\n")) { + if (line === "") { + lines.push(line) + continue + } + // stop at next top-level list item under steps (two-space + "- ") + if (/^ {0,6}- name:/.test(line) || (/^\S/.test(line) && lines.length > 0)) break + lines.push(line) + } + // Join continuation backslashes then collapse whitespace for the kilo run line + const joined = lines + .map((l) => l.replace(/^\s+/, "")) + .join("\n") + .replace(/\\\n/g, " ") + .replace(/\s+/g, " ") + assert.match(joined, /kilo run\b/, `expected kilo run in Fix verify block:\n${joined}`) + const kiloCmd = joined.match(/kilo run\b[^|]*/)?.[0] ?? "" + assert.ok( + /\s--auto\b/.test(kiloCmd) || /kilo run\s+--auto\b/.test(kiloCmd), + `Fix verify kilo run must contain --auto; got: ${kiloCmd}`, + ) + + // The step runs under `set -o pipefail` + the default `bash -e`, so an + // unguarded kilo pipeline aborts the block before verify2.log is written + // once the CLI exits nonzero on a mid-stream error. The rebuild must decide + // this step's outcome, not the agent's exit code. + // Window is the end of the kilo pipeline → the rebuild, so a comment + // elsewhere in the block cannot satisfy the guard assertion. + const teeIdx = joined.indexOf("tee -a docs-sync-out/edit-log.txt") + assert.ok(teeIdx >= 0, `expected the kilo pipeline to tee edit-log.txt:\n${joined}`) + const kiloPipeline = joined.slice(teeIdx, joined.indexOf("bun run", teeIdx)) + assert.match( + kiloPipeline, + /\|\|\s*(echo|true)\b/, + `Fix verify kilo pipeline must be guarded (|| echo/true) so bash -e cannot skip the rebuild; got: ${kiloPipeline}`, + ) + assert.match(joined, /verify2\.log/, "Fix verify block must still write verify2.log") + } + + // (iii) authoritative: real stub invocations with callLog — every argv has --auto + { + const prs = [1, 2, 3, 4, 5].map((n) => samplePr(n)) + const worthy = prs + const triage = prs.map((p) => ({ + pr: p.number, + url: p.url, + docs_worthy: true, + reason: "needs docs", + target_sections: ["overview"], + priority: "high", + })) + const cwd = setupEditCwd(worthy, triage) + const callLog = path.join(cwd, "kilo-calls.log") + const stderrText = "event stream disconnected DIAG-AUTO" + const kiloDir = makeStubKiloDir({ mode: "stderr-exit0", stderrText, callLog }) + + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + }, + }) + assert.equal(result.status, 0, `edit.mjs exit: ${result.output}`) + + assert.ok(fs.existsSync(callLog), "callLog must be written (stub was invoked)") + const lines = fs.readFileSync(callLog, "utf8").trim().split("\n").filter(Boolean) + assert.ok(lines.length > 0, "callCount > 0 required (vacuous empty log forbidden)") + for (const line of lines) { + const { argv } = JSON.parse(line) + assert.ok( + Array.isArray(argv) && argv.includes("--auto"), + `every kilo argv must include --auto; got ${JSON.stringify(argv)}`, + ) + } + } +} + +// --------------------------------------------------------------------------- +// Case 2c — full child stderr always written (success and failure paths) +// --------------------------------------------------------------------------- +function case2c_stderrLogAlways() { + console.log("case 2c: unconditional kilo-stderr-*.log") + + const prs = [1, 2, 3, 4, 5].map((n) => samplePr(n)) + const worthy = prs + const triage = prs.map((p) => ({ + pr: p.number, + url: p.url, + docs_worthy: true, + reason: "needs docs", + target_sections: ["overview"], + priority: "high", + })) + + // Failure path: stub exits 0 without summary (same mode as case 2) + { + const cwd = setupEditCwd(worthy, triage) + const stderrText = "FAILPATH-STDERR-MARKER" + const kiloDir = makeStubKiloDir({ mode: "stderr-exit0", stderrText }) + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + }, + }) + assert.equal(result.status, 0, result.output) + const logName = kiloStderrLogName("edit batch 0 attempt 1") + const logPath = path.join(cwd, "docs-sync-out", logName) + assert.equal(logName, "kilo-stderr-edit-batch-0-attempt-1.log") + assert.ok(fs.existsSync(logPath), `expected ${logPath} on failure path`) + assert.match(fs.readFileSync(logPath, "utf8"), /FAILPATH-STDERR-MARKER/) + } + + // Success path: stub writes summary (today's path that discarded stderr) + { + const cwd = setupEditCwd(worthy, triage) + const stderrText = "SUCCESSPATH-STDERR-MARKER" + const kiloDir = makeStubKiloDir({ mode: "write-edit-summary", stderrText }) + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + }, + }) + assert.equal(result.status, 0, result.output) + assert.ok( + fs.existsSync(path.join(cwd, "docs-sync-out", "edit-summary-0.json")), + "stub must write summary (success path)", + ) + const logName = kiloStderrLogName("edit batch 0 attempt 1") + const logPath = path.join(cwd, "docs-sync-out", logName) + assert.ok(fs.existsSync(logPath), `expected ${logPath} on success path`) + assert.match(fs.readFileSync(logPath, "utf8"), /SUCCESSPATH-STDERR-MARKER/) + } +} + +// --------------------------------------------------------------------------- +// Case 2d — redact secret env values from captured kilo stderr (artifact-safe) +// --------------------------------------------------------------------------- +function case2d_redactEnvSecrets() { + console.log("case 2d: redact env secrets from kilo stderr capture") + + const prs = [1, 2, 3, 4, 5].map((n) => samplePr(n)) + const worthy = prs + const triage = prs.map((p) => ({ + pr: p.number, + url: p.url, + docs_worthy: true, + reason: "needs docs", + target_sections: ["overview"], + priority: "high", + })) + const cwd = setupEditCwd(worthy, triage) + const secret = "selftest-secret-value-12345" + const stderrText = `leak before ${secret} after` + const kiloDir = makeStubKiloDir({ mode: "stderr-exit0", stderrText }) + + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + KILO_API_KEY: secret, + }, + }) + assert.equal(result.status, 0, `edit.mjs exit: ${result.output}`) + + const logName = kiloStderrLogName("edit batch 0 attempt 1") + const logPath = path.join(cwd, "docs-sync-out", logName) + assert.ok(fs.existsSync(logPath), `expected ${logPath}`) + const logBody = fs.readFileSync(logPath, "utf8") + assert.ok(!logBody.includes(secret), `persisted stderr must not contain secret; got: ${logBody}`) + assert.ok(logBody.includes("leak before *** after"), `persisted stderr must redact to exact line; got: ${logBody}`) + + // Console stderr-tail region must also be redacted (not only the artifact file). + const tailIdx = result.output.indexOf("stderr tail:") + assert.ok(tailIdx >= 0, `expected stderr tail: in output; got: ${result.output}`) + const tailRegion = result.output.slice(tailIdx) + assert.ok(!tailRegion.includes(secret), `console stderr tail must not contain secret; got: ${tailRegion}`) +} + +// --------------------------------------------------------------------------- +// Case 2e — longer secret first when a shorter env value is a prefix +// --------------------------------------------------------------------------- +function case2e_prefixSecretOrdering() { + console.log("case 2e: prefix-secret ordering (longer value redacted first)") + + const prs = [1, 2, 3, 4, 5].map((n) => samplePr(n)) + const worthy = prs + const triage = prs.map((p) => ({ + pr: p.number, + url: p.url, + docs_worthy: true, + reason: "needs docs", + target_sections: ["overview"], + priority: "high", + })) + const cwd = setupEditCwd(worthy, triage) + const shortSecret = "abcdefgh" + const longSecret = "abcdefghIJKL-tail" + const stderrText = `leak: ${longSecret} end` + const kiloDir = makeStubKiloDir({ mode: "stderr-exit0", stderrText }) + + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + A_KEY: shortSecret, + B_TOKEN: longSecret, + }, + }) + assert.equal(result.status, 0, `edit.mjs exit: ${result.output}`) + + const logName = kiloStderrLogName("edit batch 0 attempt 1") + const logPath = path.join(cwd, "docs-sync-out", logName) + assert.ok(fs.existsSync(logPath), `expected ${logPath}`) + const logBody = fs.readFileSync(logPath, "utf8") + assert.ok(!logBody.includes("IJKL-tail"), `must not leak prefix remainder; got: ${logBody}`) + assert.ok(logBody.includes("leak: *** end"), `expected full long secret redacted; got: ${logBody}`) +} + +// --------------------------------------------------------------------------- +// Case 2f — redact secret values from captured kilo stdout (triage-raw artifact) +// --------------------------------------------------------------------------- +function case2f_redactStdout() { + console.log("case 2f: redact env secrets from kilo stdout (triage-raw)") + + const digest = [samplePr(501), samplePr(502)] + const cwd = setupTriageCwd(digest) + const secret = "selftest-stdout-secret-99999" + const kiloDir = makeStubKiloDir({ mode: "triage-embed-env-secret" }) + const summaryFile = path.join(cwd, "step-summary.md") + fs.writeFileSync(summaryFile, "") + + const result = runNodeScript(TRIAGE_SCRIPT, { + cwd, + kiloDir, + env: { + TRIAGE_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + TRIAGE_BUDGET_MINUTES: "30", + GITHUB_STEP_SUMMARY: summaryFile, + KILO_API_KEY: secret, + }, + }) + assert.equal(result.status, 0, `triage.mjs exit: ${result.output}`) + + const rawFiles = fs.readdirSync(path.join(cwd, "docs-sync-out")).filter((f) => f.startsWith("triage-raw-")) + assert.ok(rawFiles.length > 0, "expected triage-raw-*.txt artifact") + for (const f of rawFiles) { + const body = fs.readFileSync(path.join(cwd, "docs-sync-out", f), "utf8") + assert.ok(!body.includes(secret), `triage-raw must not contain secret; ${f}: ${body}`) + } + + const triage = JSON.parse(fs.readFileSync(path.join(cwd, "docs-sync-out", "triage.json"), "utf8")) + assert.ok(triage.length >= 1, "triage must still parse after redaction") + assert.ok( + triage.some((e) => e.docs_worthy === true || e.pending === true || e.docs_worthy === false), + "triage entries must be structured", + ) +} + +// --------------------------------------------------------------------------- +// Case 2g — redact-stream.mjs line-wise filter (including partial last line) +// --------------------------------------------------------------------------- +function case2g_redactStream() { + console.log("case 2g: redact-stream.mjs stdin filter") + + const secret = "stream-secret-value-xyz" + const filterPath = path.join(HERE, "redact-stream.mjs") + assert.ok(fs.existsSync(filterPath), `expected ${filterPath}`) + + const input = `leak ${secret} after\npartial-${secret}` + const result = spawnSync(process.execPath, [filterPath], { + env: { ...process.env, KILO_API_KEY: secret }, + input, + encoding: "utf8", + timeout: 10_000, + }) + assert.equal(result.status, 0, `redact-stream exit: ${result.stderr || result.error}`) + assert.equal(result.stdout, "leak *** after\npartial-***") +} + +// --------------------------------------------------------------------------- +// Case 2h — pending causes reach the rolling PR free of ANSI escapes +// --------------------------------------------------------------------------- +function case2h_pendingCauseIsReadable() { + console.log("case 2h: pending cause has no ANSI escapes") + + const prs = [1, 2, 3, 4, 5].map((n) => samplePr(n)) + const worthy = prs + const triage = prs.map((p) => ({ + pr: p.number, + url: p.url, + docs_worthy: true, + reason: "needs docs", + target_sections: ["overview"], + priority: "high", + })) + const cwd = setupEditCwd(worthy, triage) + // Verbatim shape of a real kilo TUI stderr line (see PR #12521's pending table). + const ESC = "\u001b" + const stderrText = `${ESC}[0m→ ${ESC}[0mRead packages/kilo-docs/AGENTS.md${ESC}[2K${ESC}[1G done` + const kiloDir = makeStubKiloDir({ mode: "stderr-exit0", stderrText }) + + const result = runNodeScript(EDIT_SCRIPT, { + cwd, + kiloDir, + env: { + EDIT_MODEL: "test/model", + DOCS_SYNC_BACKOFF_MS: "0", + EDIT_BUDGET_MINUTES: "5", + EDIT_BATCH_TIMEOUT_MINUTES: "1", + }, + }) + assert.equal(result.status, 0, `edit.mjs exit: ${result.output}`) + + const summary = JSON.parse(fs.readFileSync(path.join(cwd, ".docs-sync-summary.json"), "utf8")) + assert.equal(summary.length, 5) + for (const e of summary) { + assert.equal(e.action, "pending", `expected pending, got ${JSON.stringify(e)}`) + assert.ok(!e.reason.includes(ESC), `pending reason must not contain ANSI escapes: ${JSON.stringify(e.reason)}`) + // Non-vacuous: the diagnostic text itself must survive the strip. + assert.match(e.reason, /Read packages\/kilo-docs\/AGENTS\.md/) + } + + // The raw artifact log keeps the escapes — it is the debugging record. + const rawLog = fs.readFileSync(path.join(cwd, "docs-sync-out", "kilo-stderr-edit-batch-0-attempt-1.log"), "utf8") + assert.ok(rawLog.includes(ESC), "persisted stderr log must stay raw") +} + +// --------------------------------------------------------------------------- +// Case 2i — wall-clock budgets can actually fit work +// --------------------------------------------------------------------------- +/** + * The pre-unit gates in triage.mjs/edit.mjs refuse to start a chunk/batch unless + * a whole per-unit timeout remains, so a budget below that timeout silently runs + * ZERO units and defers every PR. Run 30306629290 hit the weaker form of this: + * 8 of 11 chunks and 4 of 11 batches ran, the rest deferred untried. Assert the + * workflow sets both budgets and that each fits at least two units. + */ +function case2i_budgetsFitWork() { + console.log("case 2i: triage/edit budgets fit at least two units") + + const yml = fs.readFileSync(path.join(HERE, "..", "workflows", "docs-sync.yml"), "utf8") + const readEnvNumber = (key) => { + const m = yml.match(new RegExp(`^\\s*${key}:\\s*"?(\\d+)"?\\s*$`, "m")) + assert.ok(m, `${key} must be set in docs-sync.yml (default is too small to drain a backlog)`) + return Number(m[1]) + } + + // Per-unit timeouts are script constants, not workflow env; read them from source. + const triageSrc = fs.readFileSync(path.join(HERE, "triage.mjs"), "utf8") + const chunkMin = Number(triageSrc.match(/CHUNK_TIMEOUT_MS = (\d+) \* 60 \* 1000/)?.[1]) + assert.ok(Number.isFinite(chunkMin), "could not read CHUNK_TIMEOUT_MS from triage.mjs") + + const editSrc = fs.readFileSync(path.join(HERE, "edit.mjs"), "utf8") + const batchMin = Number(editSrc.match(/EDIT_BATCH_TIMEOUT_MINUTES\) \|\| (\d+)/)?.[1]) + assert.ok(Number.isFinite(batchMin), "could not read EDIT_BATCH_TIMEOUT_MINUTES default from edit.mjs") + + const triageBudget = readEnvNumber("TRIAGE_BUDGET_MINUTES") + const editBudget = readEnvNumber("EDIT_BUDGET_MINUTES") + assert.ok( + triageBudget >= 2 * chunkMin, + `TRIAGE_BUDGET_MINUTES=${triageBudget} must be >= 2x chunk timeout (${chunkMin}m)`, + ) + assert.ok(editBudget >= 2 * batchMin, `EDIT_BUDGET_MINUTES=${editBudget} must be >= 2x batch timeout (${batchMin}m)`) + + // The job timeout must outlast both budgets plus the non-LLM steps. + const jobTimeout = Number(yml.match(/^\s*timeout-minutes:\s*(\d+)\s*$/m)?.[1]) + assert.ok(Number.isFinite(jobTimeout), "could not read job timeout-minutes") + assert.ok( + jobTimeout > triageBudget + editBudget, + `job timeout-minutes=${jobTimeout} must exceed triage+edit budgets (${triageBudget}+${editBudget})`, + ) +} + // --------------------------------------------------------------------------- // Case 3 — watermark invariant // --------------------------------------------------------------------------- @@ -611,14 +1093,8 @@ function case4_routing() { triage: [], uncovered: [], }) - assert.ok( - !forgedRows.skippedRows[0].includes(""), - "clean() must strip --> from reasons", - ) + assert.ok(!forgedRows.skippedRows[0].includes(""), "clean() must strip --> from reasons") const forgedBody = renderBody({ date: "2026-07-27", since: "s", @@ -934,6 +1410,14 @@ function main() { const cases = [ case1_mergeOrFallback, case2_defectB, + case2b_autoFlag, + case2c_stderrLogAlways, + case2d_redactEnvSecrets, + case2e_prefixSecretOrdering, + case2f_redactStdout, + case2g_redactStream, + case2h_pendingCauseIsReadable, + case2i_budgetsFitWork, case3_watermark, case4_routing, case5_recollection, diff --git a/.github/docs-sync/triage.mjs b/.github/docs-sync/triage.mjs index eb075980b51d..a01eef0a9989 100644 --- a/.github/docs-sync/triage.mjs +++ b/.github/docs-sync/triage.mjs @@ -71,8 +71,13 @@ function triageChunk(chunk, index, budgetDeadline) { break } + // Headless `kilo run` auto-rejects every permission ask; without --auto the + // agent cannot run shell commands. SECURITY: --auto grants unrestricted bash + // to an agent steered by external PR content. Hardening deferred: a scoped + // permission.bash map via KILO_CONFIG_CONTENT should replace --auto once the + // required shell patterns are stable (see PR #12605 review thread). const result = runKilo({ - args: ["run", prompt, "-m", model, "--dir", process.cwd(), "-f", chunkFile], + args: ["run", "--auto", prompt, "-m", model, "--dir", process.cwd(), "-f", chunkFile], timeoutMs: Math.min(CHUNK_TIMEOUT_MS, left), streamStdout: false, label: `triage chunk ${index} attempt ${attempt}`, diff --git a/.github/workflows/docs-sync.yml b/.github/workflows/docs-sync.yml index 5a32e8769866..147cde8a4744 100644 --- a/.github/workflows/docs-sync.yml +++ b/.github/workflows/docs-sync.yml @@ -66,8 +66,13 @@ jobs: sync: if: github.repository == 'Kilo-Org/kilocode' && github.event_name != 'pull_request' runs-on: blacksmith-4vcpu-ubuntu-2404 - # Budget: 3 setup/collect + 35 triage + 50 edit + 2 verify + 10 fix + 2 upsert = 102 min, 18-minute reserve. - timeout-minutes: 120 + # Budget: 4 setup/collect + 90 triage + 120 edit + 2 verify + 10 fix + 2 upsert = 228 min, 12-minute reserve. + # These are ceilings, not costs: a caught-up run triages ~2 chunks and edits + # ~1 batch and finishes in ~25 min. The old 35/50 pair was the binding + # constraint on backlog drain — run 30306629290 deferred 54 PRs untriaged and + # 31 unedited purely on budget, with no attempt made. See the throughput note + # in the PR description for the arithmetic. + timeout-minutes: 240 env: # Both are required: without KILO_ORG_ID the gateway bills the key # owner's personal balance (402 "Add credits") instead of the org. @@ -116,6 +121,9 @@ jobs: if: steps.collect.outputs.count != '0' env: SINCE_OVERRIDE: ${{ steps.wm.outputs.since_override }} + # Default 35 fit only 8 of 11 chunks on a 254-PR window. Headroom for + # --auto making chunks slower now that the agent really runs commands. + TRIAGE_BUDGET_MINUTES: "90" run: node .github/docs-sync/triage.mjs - name: Filter docs-worthy PRs @@ -153,6 +161,11 @@ jobs: - name: Update docs (Kilo CLI, batched) if: (steps.worthy.outputs.count || '0') != '0' && inputs.dry_run != true continue-on-error: true + env: + # Default 50 fit only 4 of 11 batches. A healthy --auto batch is ~8 min, + # and edit.mjs will not start a batch without EDIT_BATCH_TIMEOUT_MINUTES + # (15) left, so 120 covers 14 batches = 70 PRs against ~5 worthy/day. + EDIT_BUDGET_MINUTES: "120" run: node .github/docs-sync/edit.mjs - name: Verify docs build and tests @@ -174,9 +187,19 @@ jobs: NEXT_PUBLIC_POSTHOG_KEY: ${{ secrets.POSTHOG_API_KEY }} run: | set -o pipefail - kilo run "The docs build or tests failed. Read the attached docs-sync-out/verify.log and fix the packages/kilo-docs changes so they pass. Do not revert doc edits; fix them. Do not modify anything outside packages/kilo-docs." \ + # Headless kilo run auto-rejects every permission ask; the runner has no + # user config granting bash, so without --auto the agent cannot run ordinary + # shell commands against the repository. + kilo run --auto "The docs build or tests failed. Read the attached docs-sync-out/verify.log and fix the packages/kilo-docs changes so they pass. Do not revert doc edits; fix them. Do not modify anything outside packages/kilo-docs." \ -m "$EDIT_MODEL" --dir "$GITHUB_WORKSPACE" -f docs-sync-out/verify.log \ - | tee -a docs-sync-out/edit-log.txt + | node .github/docs-sync/redact-stream.mjs \ + | tee -a docs-sync-out/edit-log.txt \ + || echo "::warning::kilo fix pass exited nonzero; re-verifying anyway" + # The rebuild below decides this step's outcome, not the agent's exit code. + # Without the guard above, `set -o pipefail` + the default `bash -e` would + # abort here once the CLI half of this PR ships: a mid-stream session error + # (or an auto-rejected ask) exits 1, verify2.log is never written, and + # `Re-verify status` reports VERIFIED=false even when the docs build fine. { bun run --filter @kilocode/kilo-docs build && bun run --filter @kilocode/kilo-docs test; } 2>&1 | tee docs-sync-out/verify2.log - name: Re-verify status diff --git a/packages/opencode/src/cli/cmd/run.ts b/packages/opencode/src/cli/cmd/run.ts index 56241ace1a1d..bdf726cf0abb 100644 --- a/packages/opencode/src/cli/cmd/run.ts +++ b/packages/opencode/src/cli/cmd/run.ts @@ -726,6 +726,7 @@ export const RunCommand = effectCmd({ const MAX_RETRIES = 3 // kilocode_change let retries = 0 // kilocode_change let error: string | undefined + let autoRejected = false // kilocode_change - plain headless auto-reject must fail the run // kilocode_change start - revert to upstream: consume native events without normalizing sync copies for await (const event of events.stream) { @@ -816,8 +817,10 @@ export const RunCommand = effectCmd({ err = String(props.error.data.message) } error = error ? error + EOL + err : err - if (emit("error", { error: props.error })) continue + // kilocode_change start - stderr first so --format json still surfaces the diagnostic UI.error(err) + emit("error", { error: props.error }) + // kilocode_change end } // kilocode_change start - reset retry budget only after resumed work becomes busy @@ -868,6 +871,7 @@ export const RunCommand = effectCmd({ UI.Style.TEXT_NORMAL + `subagent permission requested: ${permission.permission} (${permission.patterns.join(", ")}); auto-rejecting`, ) + autoRejected = true // kilocode_change await client.permission.reply({ requestID: permission.id, reply: "reject", @@ -889,6 +893,7 @@ export const RunCommand = effectCmd({ UI.Style.TEXT_NORMAL + `permission requested: ${permission.permission} (${permission.patterns.join(", ")}); auto-rejecting`, ) + autoRejected = true // kilocode_change await client.permission.reply({ requestID: permission.id, reply: "reject", @@ -915,6 +920,14 @@ export const RunCommand = effectCmd({ } // kilocode_change end } + // kilocode_change start - idle must not clear an auto-rejected headless run + if (autoRejected) { + const msg = "run ended with an auto-rejected permission; pass --auto for autonomous use" + error = error ? error + EOL + msg : msg + UI.error(msg) + emit("error", { error: msg }) + } + // kilocode_change end return error } const cwd = args.attach ? (directory ?? sess.directory ?? (await current(sdk))) : (directory ?? root) diff --git a/packages/opencode/test/cli/run/run-process.test.ts b/packages/opencode/test/cli/run/run-process.test.ts index 9d2d013ac096..f50ad1c796a4 100644 --- a/packages/opencode/test/cli/run/run-process.test.ts +++ b/packages/opencode/test/cli/run/run-process.test.ts @@ -47,19 +47,75 @@ describe("opencode run (non-interactive subprocess)", () => { // kilocode_change end // kilocode_change start - // Locks in the current behavior: when the LLM stream errors mid-response - // (the prompt was accepted, then the upstream provider failed), opencode - // emits a session.error event and the process exits 0 today. - // - // This is debatable — a future cleanup might flip it to exit 1. If you're - // changing this expectation, do it deliberately and say so in the PR. + // Was: "mid-stream LLM error still exits 0 today (contract lock-in)" using + // llm.fail(...). That fixture never published a session.error the CLI + // consumed (stream failure recovered as incomplete; idle with error unset), + // so the lock-in was locking in a false premise. Contract change: a run + // whose session actually errored mid-stream must exit non-zero with a + // stderr diagnostic naming the cause — callers (docs-sync) cannot otherwise + // distinguish success from a dead run and had to stop trusting exit codes. cliIt.live( - "mid-stream LLM error still exits 0 today (contract lock-in)", + "mid-stream session error exits nonzero with a stderr diagnostic", ({ llm, opencode }) => Effect.gen(function* () { - yield* llm.fail("upstream provider exploded mid-stream") + yield* llm.error(400, { error: { message: "upstream provider exploded mid-stream" } }) const result = yield* opencode.run("trigger midstream error", { timeoutMs: 30_000 }) - expect(result.exitCode).toBe(0) + opencode.expectExit(result, 1) + expect(result.stderr).toContain("upstream provider exploded mid-stream") + }), + 60_000, + ) + + cliIt.live( + "mid-stream session error exits nonzero with stderr diagnostic under --format json", + ({ llm, opencode }) => + Effect.gen(function* () { + yield* llm.error(400, { error: { message: "upstream provider exploded mid-stream" } }) + const result = yield* opencode.run("trigger midstream error", { + format: "json", + timeoutMs: 30_000, + }) + opencode.expectExit(result, 1) + expect(result.stderr).toContain("upstream provider exploded mid-stream") + const events = opencode.parseJsonEvents(result.stdout) + expect(events.some((event) => event.type === "error")).toBe(true) + }), + 60_000, + ) + + // Plain headless run (no --auto): CLI auto-rejects bash asks under the + // harness's isolated config. Any auto-reject ⇒ non-zero, even if idle. + cliIt.live( + "auto-rejected permission in plain headless run exits nonzero", + ({ llm, opencode }) => + Effect.gen(function* () { + yield* llm.tool("bash", { command: "sed -n 1,5p README.md" }) + const result = yield* opencode.run("run sed on readme", { timeoutMs: 45_000 }) + opencode.expectExit(result, 1) + expect(result.stderr).toContain("permission requested: bash") + expect(result.stderr).toContain("auto-rejecting") + expect(result.stderr).toContain( + "run ended with an auto-rejected permission; pass --auto for autonomous use", + ) + }), + 60_000, + ) + + cliIt.live( + "auto-rejected permission exits nonzero with stderr diagnostic under --format json", + ({ llm, opencode }) => + Effect.gen(function* () { + yield* llm.tool("bash", { command: "sed -n 1,5p README.md" }) + const result = yield* opencode.run("run sed on readme", { + format: "json", + timeoutMs: 45_000, + }) + opencode.expectExit(result, 1) + expect(result.stderr).toContain( + "run ended with an auto-rejected permission; pass --auto for autonomous use", + ) + const events = opencode.parseJsonEvents(result.stdout) + expect(events.some((event) => event.type === "error")).toBe(true) }), 60_000, ) @@ -76,6 +132,7 @@ describe("opencode run (non-interactive subprocess)", () => { yield* llm.text("structured output") const result = yield* opencode.run("say hi", { format: "json", extraArgs: ["--auto"] }) opencode.expectExit(result, 0) + expect(result.stdout).not.toContain("auto-rejected permission") const events = opencode.parseJsonEvents(result.stdout) expect(events.length).toBeGreaterThan(0)