feat(cloud-agent): new worker for new kilo cli based on focus-week branch#14
Conversation
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
Same caveat about kilo review items as on the original PR. They're valid, but trying to avoid introducing new changes in this pr since this pr is mostly existing code. I can idle a cloud agent to fix items kilo review flagged after this lands. |
There was a problem hiding this comment.
This is probably the most important bit of this whole PR and the only thing that really warrants close 👁️ by reviewers.
Big thing to look out for is accidental references to the same resources as the original cloud-agent worker.
name is unique/new
dev.port is unique across the repo
workers using a new subdomain
callback queue is a new one
| "preview_urls": false, | ||
| "workers_dev": true, | ||
| "dev": { | ||
| "port": 8794, |
There was a problem hiding this comment.
i actually found an issue in the current repo: "port": 8792 exists in 2 wrangler.jsonc files.
There was a problem hiding this comment.
But this is using 8794 ?
There was a problem hiding this comment.
i mean its not a problem for this PR. I just found this as an existing issue.
| "containers": [ | ||
| { | ||
| "class_name": "Sandbox", | ||
| "image": "./Dockerfile.dev", |
There was a problem hiding this comment.
maybe this is too nitpicky but these dockerfile names can be confusing.
Dockerfile => old CLI
Dockerfile.next => new CLI
There was a problem hiding this comment.
else then it should be more obvious between the wrangler.jsonc of cloud-agent in the KILOCODE_CLI_VERSION should be the old version and in here the next (as it is done)
There was a problem hiding this comment.
Theres no distinction between old and new cli in this worker. Its only new cli.
dagadbm
left a comment
dagadbm
left a comment
There was a problem hiding this comment.
i left some non blocking comments in wrangler.jsonc file.
I approved to not block but I dont have enough context to decide if there are other things that should be looked at more carefully.
|
|
||
| export async function getCurrentBranch(workspacePath: string): Promise<string> { | ||
| try { | ||
| const result = await exec(`cd ${workspacePath} && git branch --show-current`); |
There was a problem hiding this comment.
WARNING: Potential shell injection / path breakage via unquoted workspacePath
exec() runs via sh -c, and this line interpolates workspacePath without quoting/escaping. If workspacePath can be influenced (or even just contains spaces), the command can break or allow arbitrary shell execution. Also note exec() resolves on non-zero exit, so failures won’t hit the catch here.
Consider avoiding sh -c entirely and running git with spawn(..., { cwd: workspacePath }), or at minimum properly quote/escape and check exitCode.
Executed via three sub-agent runs: 1. Browse (Flow 2) — sub-agent verified browse-direct matches upstream count 2. Full lifecycle (Flows 3, 4, 6, 7) — sub-agent verified post -> claim -> done -> accept on item w-870be07fbc through PRs #8-11. Stamp s-35e8a923... issued with author=jrf0110, subject=jfawcett. 3. Branches (Flows 5, 8, 9) — sub-agent verified unclaim (item w-68aa4ab1dd, PR #14), reject (w-d2cf6acf6a, PR #18), and close (w-89e6720ca4, PR #22) on fresh items. Only Flow 10 (disconnect) remains — it is a pure gastown operation and doesn't touch upstream DoltHub state, so lower priority. Total: 19 PRs merged, 4 rigs in play (jfawcett contributor, jrf0110 maintainer), full lifecycle graph exercised.
2 participants
This is the same as the https://github.com/Kilo-Org/kilocode-backend/tree/kilo branch we've been hacking on since focus-week, just with the cloud-agent v2 routes changes broken out into a new dedicated worker cloud-agent-next.
The only difference is that this does also have the userId encoding fix that landed in main
todayyesterday, and that this worker has none of the legacy routes.https://www.loom.com/share/9ee9ace2883f4dc9ae8266e3c3d339e8