fix(ci): docs-sync bot passes --auto, drains its backlog, and reports readable causes; fix(cli): honest exit codes for headless runs - #12605
fix(ci): docs-sync bot passes --auto, drains its backlog, and reports readable causes; fix(cli): honest exit codes for headless runs#12605iscekic wants to merge 9 commits into
Conversation
…logs Headless kilo run auto-rejects every permission ask it receives, and the GitHub runner has no user config granting bash — so without --auto the docs-sync bot's triage, edit and verify-fix calls were silently crippled whenever the agent reached for a non-allowlisted shell command (CI run 30306629290: 9 rejections, all 11 edit batches failed, exit 0). - Pass --auto immediately after "run" at all three call sites (triage.mjs, edit.mjs, docs-sync.yml Fix verify failures step) - runKilo now always writes the child's full stderr to docs-sync-out/kilo-stderr-<label>.log, on success as well as failure — the blindness that hid the defect - selftest asserts --auto non-vacuously (region-scoped source match + a stub invocation that records argv) and proves the stderr log is written on both the failure and the summary-writing success path
…n errors Non-interactive kilo run reported success for runs that accomplished nothing — a caller cannot distinguish success from a dead run, which is why the docs-sync bot had to stop trusting exit codes entirely. - Plain headless run (neither --auto nor --dangerously-skip-permissions) in which the CLI auto-rejected at least one permission ask now exits non-zero, even when the session still reaches idle afterwards, with the diagnostic: run ended with an auto-rejected permission; pass --auto for autonomous use. Deliberate contract change: any auto-rejected ask means the run was crippled, not successful. - A mid-stream session error now writes its diagnostic to stderr before emitting the json error event, so the cause is visible under --format json as well (the emit-first shape skipped the stderr write). - The old exit-0 contract lock-in test is replaced: its llm.fail fixture never published a consumable session.error, so it locked in a false premise. New tests cover both scenarios in both output formats; happy-path and --format json runs still exit 0 unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Resolved since last review:
The rest of the incremental diff (i18n files, Agent Manager terminal work, nix/flake updates, etc.) came in via the merge commit from Fix these issues in Kilo Cloud Files Reviewed (3 files)
Previous Review Summary (commit 20b058b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 20b058b)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
The CLI-side logic in Fix these issues in Kilo Cloud Files Reviewed (7 files)
Reviewed by claude-sonnet-5 · Input: 44 · Output: 8K · Cached: 1.4M Review guidance: REVIEW.md from base branch |
The full-stderr capture added for observability lands in uploaded CI artifacts, which are raw files (GitHub masks secrets in log streams only) on a public repo, and the runner env holds a long-lived KILO_API_KEY. Redact exact values of KEY|TOKEN|SECRET-named env vars (len >= 8) once at capture, so both the console tail and the artifact file are safe. Adds the changeset for the headless exit-code contract change.
|
(bot) @kilocode-bot please review the latest head (090dfd0). Changes since the last review: merged origin/main, redacted KEY|TOKEN|SECRET env values from the persisted kilo stderr logs (addresses the lib.mjs thread), added the exit-code-contract changeset. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review from The two previously open findings ( Files Reviewed (incremental: 2 files across 1 new commit)
Previous Review Summaries (4 snapshots, latest commit 3b32193)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3b32193)Status: No Issues Found | Recommendation: Merge Incremental review from The two previously open findings are unaffected by this commit and were already addressed on their own threads: the Files Reviewed (incremental: 3 files across 1 new commit)
Previous review (commit 88b2442)Status: No Issues Found | Recommendation: Merge Incremental review from This directly addresses the previously open WARNING on Files Reviewed (incremental: 2 files across 1 new commit)
Previous review (commit 988a1d8)Status: No Issues Found | Recommendation: Merge Incremental review from
No new issues found in the incremental diff. The two previously-open WARNING threads (unconditional stderr capture blast radius; Files Reviewed (incremental: 4 files across 2 new commits)
Previous review (commit 090dfd0)Status: 1 Issue Found (incremental review of Overview
Issue Details (click to expand)WARNING
SUGGESTION
Verification of the three new changes:
Still open (informational, unchanged):
CI note: the only red job is Files Reviewed (3 files + ancestry)
Reviewed by claude-sonnet-5 · Input: 36 · Output: 7.7K · Cached: 855.2K Review guidance: REVIEW.md from base branch |
|
The Cloud Agent already posted the review to the PR: #12605 (comment) Summary: 0 CRITICAL, 1 WARNING (stdout redaction gap in No further action needed unless you want me to spawn a follow-up agent to fix the stdout redaction gap. |
The docs-sync workflow uploads docs-sync-out/ as a public 14-day artifact, and kilo stdout was persisted raw there in two more places: triage-raw-*.txt and the edit-log.txt tee. Redact at capture in runKilo for stdout as already done for stderr, and pipe the verify-fix step's kilo stdout through a new line-wise redact-stream.mjs filter before tee. Also harden redactEnvSecrets: widen the name pattern to KEY|TOKEN|SECRET|CREDENTIAL|PASSWORD|ORG_ID|_PAT (KILO_ORG_ID is a repo secret), replace longer values first so a short secret that prefixes a longer one cannot leak the remainder, and document the exact-substring limitation. Selftest gains cases 2e (prefix ordering), 2f (stdout capture), 2g (stream filter) and a non-vacuous exact-line assertion in 2d.
The auto-reject path adds a new error event to the --format json stream; only the existing event shapes are unchanged. Also state that the exit-1 rule covers a plain non-interactive --attach run that auto-rejects an ask.
Update comments in triage.mjs and edit.mjs to accurately describe the security implications of --auto (unrestricted bash for an agent steered by external PR content) and note that a scoped permission.bash map via KILO_CONFIG_CONTENT is the intended hardening, deferred until required shell patterns are stable.
|
(bot) CI triage on the latest head, for reviewers: Green: every check except the two below —
|
--auto fixes the batches the bot attempted; it does not fix the ones it never started. In run 30306629290 (254 PRs collected, 51 docs-worthy), the wall-clock budgets deferred 54 PRs untriaged and 31 unedited without an attempt — 45 of the 60 pending rows on the rolling PR. Triage got 8 of 11 chunks in 35 min; edit got 4 of 11 batches in 50 min. Both are ceilings, not costs. A caught-up run needs ~2 chunks and ~1 batch and finishes in ~25 min, so raising them spends nothing on a normal day and drains the backlog on a bad one. 90/120 covers 20 chunks and 14 batches — 500 triaged and 70 edited PRs against a ~5 docs-worthy/day inflow — inside a 240-minute job timeout. Also strip ANSI CSI sequences in tailText, the shared path both triage and edit route their pending causes through. kilo renders its TUI to stderr, so every "Why" cell on the rolling PR currently reads "^[[0m→ ^[[0mRead packages/..." instead of the diagnostic. The persisted docs-sync-out/kilo-stderr-*.log stays raw as the debugging record. selftest case 2h asserts the pending reason is escape-free and still carries the diagnostic text; case 2i asserts each budget fits at least two units and the job timeout outlasts both, so a future edit cannot silently restore a budget too small to run anything. Both shown failing on the unmodified code first.
iscekic
changed the title
The "Fix verify failures" step runs under `set -o pipefail` and the default `bash -e`. Once the CLI half of this PR ships, `kilo run` exits 1 on a mid-stream session error, which aborts the block before the rebuild runs: verify2.log is never written and `Re-verify status` reports VERIFIED=false even when the docs build fine. A transient provider error would flip every rolling docs PR to "Verification: failing". The agent's exit code was never the signal for this step — the rebuild is. Guard the pipeline with `|| echo ::warning::` so a nonzero kilo run is surfaced but the rebuild still decides the outcome. Extends selftest case 2b (which already parses this step) rather than adding a case: the guard must appear between the pipeline's tee and the rebuild, so a comment elsewhere in the block cannot satisfy it. Shown failing with the guard removed.
2 participants
Issue
Closes the CLI-side follow-up that #12580 deliberately deferred. No separate issue exists — #12580 made the docs-sync bot survive broken CLI calls (pending rows, held-back watermark, stderr capture on failure); this PR fixes the causes.
Context
The daily
docs-syncbot drives the Kilo CLI headless to updatepackages/kilo-docs. Its CLI calls were doing no work whenever the agent reached for a shell command, and nothing could see it:Headless
kilo runauto-rejects every permission ask it receives. Without--auto, a plainkilo runrepliesrejectto anybashpermission ask (run.ts). A GitHub runner has no user-level config grantingbash: allow, so ordinary read-only commands (sed -n …,git log … -- <path>,find …) are auto-rejected and the run ends without writing its summary. All three docs-sync call sites omitted--auto. In CI run 30306629290 this fired 9 times (auto-rejecting→Error: The user rejected permission…), all 11 edit batches failed across 3 attempts, 0 summary files. The defect is intermittent by model behaviour —read/edit/glob/grepare allowed outright, so a run whose model never shells out succeeds anyway — which is exactly what made it hard to see.A crippled run was indistinguishable from a healthy one. Non-interactive
kilo runexited 0 for runs that accomplished nothing: the permission-rejected case above reachedidlenormally and exited 0, and the bot also discarded the child's stderr on the success path, so a "successful" call that was auto-rejected left no trace anywhere.Throughput, not correctness, is what defers most PRs.
--autofixes the batches the bot attempted; it does not fix the ones it never started. In the same CI run 30306629290 (254 PRs collected, 51 docs-worthy), the wall-clock budgets deferred 54 PRs untriaged (triage got 8 of 11 chunks in 35 min) and 31 unedited (edit got 4 of 11 batches in 50 min) with no attempt made. That is 45 of the 60 pending rows on rolling PR docs: auto-sync with merged PRs (through 2026-07-27) #12521; the auto-reject defect accounts for the other 15. Fixing only the first two defects would leave the bot delivering ~20 PRs/run against an 11-day backlog.Pending causes are unreadable.
runKilo'sstderrTailfeeds the rolling PR's pending table, and kilo renders its TUI to stderr, so every "Why" cell reads^[[0m→ ^[[0mRead packages/...instead of the diagnostic.Implementation
Two commits, two halves of the fix:
fix(ci)— the bot's side (takes effect on merge):--autoat all three docs-sync CLI call sites (triage.mjs,edit.mjs, theFix verify failuresstep indocs-sync.yml), immediately after"run", with a comment explaining why.--autois the documented flag for autonomous/pipeline usage; the bot is scheduled-only onmain, runs on a trusted runner, never executes PR-branch code, andupsert-pr.mjsstages onlypackages/kilo-docs— which bounds what can be committed, not what the agent can execute (see the residual-risk note below).runKilonow always persists the child's output todocs-sync-out/(full stderr tokilo-stderr-<label>.log, 8 MB cap, warn-and-continue on write failure) — previously stderr surfaced only on failure, so a run that succeeded while being auto-rejected left no trace. This is what makes the rejection check observable at all. Because these logs land in uploaded CI artifacts — raw files on a public repo, and GitHub masks secrets in log streams only — all persisted output (stderr and stdout, including thetriage-raw-*.txtsnapshots) is redacted at capture withredactEnvSecrets, and theFix verify failuresstep streams throughredact-stream.mjsbeforeteereachesedit-log.txt: exact values of env vars named KEY|TOKEN|SECRET|CREDENTIAL|PASSWORD|ORG_ID|_PAT (length ≥ 8), longest-first so a shorter secret that prefixes a longer one cannot leak a remainder (selftest cases 2d–2g, proven red-first).selftest.mjsasserts all of this non-vacuously: region-scoped matchers (theargs: [ … ]array slice; the workflow step's joinedrun: |block) so a comment can't satisfy them, plus a stub-invocation case that requirescallCount > 0and--autoin every recorded argv, and two stub cases proving stderr is persisted on both the failing and the summary-writing success path.fix(cli)— honest exit codes (reaches the bot when@kilocode/cliis next published):--autonor--dangerously-skip-permissions) in which the CLI auto-rejected any permission ask now exits 1 withrun ended with an auto-rejected permission; pass --auto for autonomous useon stderr — even if the agent recovered via allowlisted tools afterwards. The blunter rule was chosen over "non-zero only when nothing was accomplished" because "accomplished" has no crisp definition, and that fuzziness is what let this failure hide for days. Correction to an earlier claim in this description: this was previously stated as "no known caller branches on this exit code". One does, indirectly — theFix verify failuresstep runs underset -o pipefailand the defaultbash -e, so a nonzerokilo runwould abort the block before the rebuild, leavingverify2.logunwritten andRe-verify statusreportingVERIFIED=falseeven when the docs build fine. Fixed here by guarding that pipeline with|| echo "::warning::…"so the rebuild stays authoritative (selftest case 2b, shown red with the guard removed). The remaining callers do not branch on it: docs-sync decides on the summary file / parseable stdout, and the kilo workflow's dispatchers read sentinel lines. This replaces a test that locked the old exit-0 behaviour in on purpose; the replacement records what it used to lock in and why the contract changed. The flag is set in both the root and the subagent auto-reject branches (same defect class; the subagent branch has no harness fixture — recorded gap, no test).--format jsonby anif (emit(...)) continueearly-out — stderr is now written before the event emit in both formats.idle) proved unreachable in the non-interactive flow and was dropped with evidence rather than faked. Thesession.error-id-mismatch widening was likewise not applied — the diagnosis showed the error event carries the run's own session id.--attach, the network-retry block, and what the permission branches approve or reject are untouched — only the exit code changes.fix(ci)— throughput (takes effect on merge):TRIAGE_BUDGET_MINUTES: "90"andEDIT_BUDGET_MINUTES: "120"on their workflow steps (both alreadyNumber(env) || defaultintriage.mjs/edit.mjs, so this is env-only), withtimeout-minutes120 → 240. These are ceilings, not costs: a caught-up run needs ~2 chunks and ~1 batch and finishes in ~25 min, so raising them spends nothing on a normal day and drains the backlog on a bad one. 90/120 covers 20 chunks and 14 batches — 500 triaged and 70 edited PRs — against a ~5 docs-worthy/day inflow, inside the 240-minute job timeout (228 planned, 12-minute reserve).--autothey do real work. 90 still covers 254 PRs even if chunk time doubles.edit.mjs:77,141reserves a fullEDIT_BATCH_TIMEOUT_MINUTES(15) before starting a batch even though a healthy batch is ~8 min — that is what turned "590s left" into 31 deferred PRs. It is a real flaw, but at a 120-minute budget the gate stops at batch 14 and only 11 are needed, so fixing it buys nothing today. Same for parallelising batches (they share one worktree) and alarming on the 14-day lookback cap (unreachable once the watermark catches up).tailText— the shared path bothtriage.mjsandedit.mjsroute their pending causes through, rather than at each call site. Stripped before the line/char slice so escapes do not consume the tail budget. The persisteddocs-sync-out/kilo-stderr-*.logstays raw; it is the debugging record.selftest.mjscase 2h asserts the pending reason in.docs-sync-summary.jsonis escape-free and still carries the diagnostic text (non-vacuous both ways), and that the artifact log stays raw. Case 2i asserts each budget is set and fits at least two units, and that the job timeout outlasts both — the invariant that matters, since a budget below one unit timeout silently runs zero units. Both shown failing on the unmodified code first (EDIT_BUDGET_MINUTES=10→ red; env removed → red).Publish-lag split, stated plainly:
docs-sync.ymlinstalls the published CLI (npm install -g @kilocode/cli; 7.4.16 in the failing runs). Thefix(ci)commit changes the bot's own scripts, so it fixes production on merge. Thefix(cli)commit changes CLI source, so the bot gets honest exit codes only after the next@kilocode/clirelease. The bot does not "now report failures honestly" before that.Residual risk, stated plainly (deferred hardening): with
--auto, an agent steered by injected PR content (merged-PR titles/bodies/diffs from a public repo, outside contributors included) can run arbitrary shell in the job.KILO_API_KEY/KILO_ORG_IDare job-level env, so the LLM steps necessarily hold them;GH_TOKENis step-scoped to the non-LLM steps only, butactions/checkout@v6defaults topersist-credentials: true, leaving a pushable credential in.git/configfor the whole job. The exfiltration channel this PR does close: every persisted/streamed kilo log is redacted at capture. Deliberately not in this PR: (a) a scopedpermission.bashmap viaKILO_CONFIG_CONTENT— the right hardening, but the agent's shell patterns are unpredictable, and on the published CLI (7.4.16) a too-narrow map silently re-creates the original no-work defect, whose honest exit codes only ship with the next release; (b)persist-credentials: false— the rolling-branch push inupsert-pr.mjsrelies on the stored credential and that path is only verifiable on a live scheduled run. Tracked in the deliberately unresolvedtriage.mjsreview thread for the human reviewer.Screenshots / Video
N/A — CI scripts and CLI process behaviour, no visual changes.
How to Test
Manual/local verification (all executed by the agent)
r0 repro gate (pre-fix, on the unmodified baseline): the deterministic local repro produced the verbatim signature
permission requested: bash (sed -n 1,5p README.md); auto-rejecting, no completed answer, exit 0; the--autocontrol ran clean (zero rejections, command executed, answer produced). This pair is reproduced post-fix as AC1a below.node .github/docs-sync/selftest.mjs— all cases pass, including the new--autoand unconditional-stderr assertions (each shown failing on unmodified code first).oxlint .github/docs-sync— 0 errors;bun run script/check-workflows.ts— ok (29 workflows).From
packages/opencode/:bun run typecheckclean;bun test ./test/cli/run/run-process.test.ts— 8 pass / 0 fail, including the new non-zero-exit cases and the unchanged happy-path /--format jsonguards.bun run script/check-opencode-annotations.ts --worktree— clean; every shared-file edit is wrapped inkilocode_changemarkers.AC1a (deterministic two-way proof, post-fix): PASS. Same argv shape
edit.mjsuses,XDG_CONFIG_HOMEisolation, a prompt that must shell out (sed -n 1,5p README.md), published CLI 7.4.16,kimi-k3 --variant high. Without--auto: exactly onepermission requested: bash (sed -n 1,5p README.md); auto-rejecting, the README lines never appear. With--auto: zeroauto-rejectinglines and the sed output is present. (One pre-agent startup crash from a local credential-store write race on the shared verification machine — classified a test-environment failure, retried clean; recorded in the workflow learnings.)AC1b (batch integration regression, post-fix): PASS. Real
.github/docs-sync/edit.mjsover the CI-run-30306629290 fixtures (5 PRs = one batch), clean start enforced.docs-sync-out/edit-summary-0.jsonproduced by this run with one entry per batch-0 PR (3updated, 2skipped; consoleedit pass complete: 3 changed, 2 skipped, 0 pending, ~6 min). The new unconditional stderr logkilo-stderr-edit-batch-0-attempt-1.log, ANSI-stripped: 0auto-rejectinglines, 0permission requested: bash/✗ … failedmarkers, 14 successful$bash renders — the agent shelled out and bash ran under--auto. AC1b alone proves only that the added argv token did not break the invocation (the baseline passes without--autowhen the model stays on allowed tools); AC1a is the proof the defect is fixed.node .github/docs-sync/selftest.mjs— all cases pass including 2h/2i; each shown red first on unmodified code.oxlint .github/docs-sync— 0 errors, 2 warnings, both pre-existing on the branch (verified by stashing).bun run script/check-workflows.ts— ok (29 workflows);bun run script/check-opencode-annotations.ts --worktree— clean.Budget arithmetic is checked by case 2i rather than asserted in prose: gates are
remaining >= unit timeout, so 90/10 → 20 chunks and 120/15 → 14 batches.Reviewer test steps
env XDG_CONFIG_HOME=$(mktemp -d) kilo run 'Run exactly this shell command and show its output: sed -n 1,5p README.md' -m kilo/moonshotai/kimi-k3 --dir "$PWD"→ observeauto-rejectingand no command output (the isolation matters: a dev machine withpermission.bash: "allow"in its user config hides the defect).--auto→ the command runs.git showthe two commits; run the selftest and therun-processtests as above.Blocked checks and substitute verification
kilobinary (7.4.16 predates it) — substitute verification is therun-processtest harness running the worktree source, per the publish-lag split above.What to watch after merge
The first scheduled
docs-syncrun after merge (07:00 UTC daily) is the real-world confirmation: batches should now produce summary files and theirdocs-sync-out/kilo-stderr-*.logartifacts will show zeroauto-rejectinglines. One measurement that must not be undone: a healthy--autobatch takes ~8 minutes (7m45s measured at the r0 gate) because the agent now really runs its shell commands — production's 15-minuteEDIT_BATCH_TIMEOUT_MINUTESdefault is correct and must not be tightened.The throughput change has its own signal: that run should defer zero PRs for "budget exhausted" and drain #12521's backlog in one pass, advancing the watermark ~11 days in a single run instead of the ~1 hour it has been managing. If chunk or batch times come in far above the ~3.3 min / ~8 min this sizing assumes, the budgets are the knob to turn, not the batch size. Note the daily runs of 07-26 → 07-28 died before the edit pass on the git-identity fault that #12580 fixed at 08:40 UTC on 07-28; the 07-29 run is the first with that fix, so it is also the first meaningful production sample.
Checklist
.changeset/headless-run-honest-exit.md,@kilocode/clipatch) for the CLI exit-code contract change; no user-facing UI changes