fix(cli): preserve --raw atoms verbatim in run handler (#9622)#9653
Conversation
| @@ -0,0 +1,11 @@ | |||
| // Atoms before `--` are positional shell arguments where re-quoting around | |||
There was a problem hiding this comment.
WARNING: New shared opencode file is missing the required Kilo annotation
This file lives under packages/opencode/ and does not have kilocode in its path, so the annotation checker treats it as shared upstream code. New shared files need the first non-empty line to be // kilocode_change - new file; without that, script/check-opencode-annotations.ts will flag every added line.
| let message = [...args.message, ...(args["--"] || [])] | ||
| .map((arg) => (arg.includes(" ") ? `"${arg.replace(/"/g, '\\"')}"` : arg)) | ||
| .join(" ") | ||
| let message = buildRunMessage(args.message, args["--"]) |
There was a problem hiding this comment.
WARNING: Shared opencode changes need kilocode_change coverage
This added line is in an upstream-owned file and is not inside a kilocode_change block or marked inline. The same applies to the new import, so the opencode annotation check will fail unless these additions are annotated or moved into a kilocode path.
| @@ -0,0 +1,36 @@ | |||
| import { describe, expect, test } from "bun:test" | |||
There was a problem hiding this comment.
WARNING: New shared opencode test file is missing the required Kilo annotation
Because this test is under packages/opencode/test/ but not a kilocode path, the annotation checker requires either a whole-file // kilocode_change - new file marker as the first non-empty line or placing the test under packages/opencode/test/kilocode/. Without that, CI will flag the added test lines.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The latest commit (
The fix logic is unchanged and correct: positionals retain wrap-quote behavior (preserving #4979), while Files Reviewed (3 files)
Reviewed by claude-sonnet-4.6 · 561,008 tokens Review guidance: REVIEW.md from base branch |
|
Friendly check-in. 16 days in with kilo-code-bot recommending merge and no human review yet. Happy to rebase if anything's drifted on main. |
Atoms in `args["--"]` are raw passthrough per yargs `populate--` semantics: the user typed `--` to opt out of further parsing, so the assembler must not synthesize quote bytes around them. Re-quoting raw atoms breaks leading-dash inputs like kilo run -- "- Who are you?" by emitting `"- Who are you?"` (literal quotes) into the model prompt. Atoms before `--` keep the existing wrap-quote behavior from Kilo-Org#4979 so shell-bound multi-word positionals still preserve their boundaries. Extracted the message assembler to `cli/cmd/run-message.ts` so it can be unit-tested without pulling in the full handler's dependency tree.
Per AGENTS.md, every Kilo-specific change in shared opencode files must carry kilocode_change markers so upstream merges can resolve mechanically. Adds whole-file annotations to the new helper and test, single-line annotations to the two modified lines in run.ts.
00c0398 to
643810c
Compare
|
Rebased onto current main. Conflicts came from the upstream |
|
Checking in. Last rebase landed on May 15, tests still green, no maintainer asks open on my side. Happy to rebase again if main has drifted; otherwise just flagging. |
Summary
Fixes #9622. When a user runs
kilo run -- "- Who are you?", the run handler emits literal-quote bytes into the model prompt, producing"- Who are you?"instead of- Who are you?. The downstream pipeline rejects the leading-quote-then-dash sequence and the command fails ("❌ Can not run" per the OP).The bug is in
packages/opencode/src/cli/cmd/run.tsat the message assembler, which post-#4979 wraps every atom containing a space in synthesized quote bytes. That intent makes sense for atoms before--(the user's shell quoting represents a word-binding boundary they want preserved), but it is wrong for atoms after--: yargs'spopulate--semantics mean the user typed--precisely to opt out of further parsing, so anything inargs["--"]is already raw passthrough.Investigation
Verified bug repro at HEAD (
be0ea1990) and tagv7.1.23(the version OP @hbrls reported on Windows 11 + PowerShell 7). Same handler at both points:The over-quote was introduced in
be8116e2("fix: preserve argument boundaries in run command (#4979)"), which addressed a different case: shell-bound multi-word positionals likekilo run hello "world foo" barlosing their boundary on the model side. That intent is preserved by this PR; only the dashDash branch changes.yargs itself parses
kilo run -- "- Who are you?"correctly (verified via standalone yargs@18 trace in the issue thread):args.message = [],args["--"] = ["- Who are you?"]. The single-atom shape from yargs is what the assembler then re-wraps incorrectly.Fix
Extract the message assembler to
packages/opencode/src/cli/cmd/run-message.tsso it can be unit-tested without pulling in the full handler's dependency tree, and split the assembly per source:Positionals (
args.message) keep the wrap-quote behavior from #4979. dashDash (args["--"]) joins verbatim.Tests
packages/opencode/test/cli/cmd/run.test.ts(new file) covers seven cases:args["--"]through verbatim without wrap-quote (The cli parse the starting dash in a prompt wrong #9622 root case)Stash-bisect proof
Reverted
run-message.tsto the pre-fix behavior (re-quote all atoms including dashDash) and ran the suite:Restored the fix, full suite green:
Validation
bun test test/cli/cmd/run.test.ts— 7 pass / 0 failbun x prettier --checkon all three changed files — cleanbun x oxlinton all three changed files — 0 errors (8 pre-existing warnings inrun.ts, none from this change)Compatibility
No public API change. The handler still accepts the same args and emits the same shape of message to the session. Only the byte-level content of the message string changes for the
args["--"]path, which is what the bug was.Closes
Closes #9622