Skip to content

Debug/windows rg ci#9498

Open
imanolmzd-svg wants to merge 161 commits intomainfrom
debug/windows-rg-ci
Open

Debug/windows rg ci#9498
imanolmzd-svg wants to merge 161 commits intomainfrom
debug/windows-rg-ci

Conversation

@imanolmzd-svg
Copy link
Copy Markdown
Contributor

Test windows CI

nexxeln and others added 30 commits April 10, 2026 09:34
rekram1-node and others added 25 commits April 23, 2026 00:29
…fect migration

Replace direct Zod schema calls (Config.Info.parse, Config.Info.safeParse,
Snapshot.FileDiff, WorktreeDiff.Item) with their .zod counterparts to
align with the Effect Schema migration that wraps schemas via withStatics.

Migrate WorktreeDiff.Item from Zod's z.object/extend to Effect's
Schema.Struct with annotate and zod bridge. Add missing return statement
in TsClient notify.open stub and widen waitForDiagnostics input type to
match updated upstream interface.
… type annotations

Add a lightweight SummaryFileDiff derived from FileDiff via
Struct.omit(["patch"]) to keep session summary DB payloads small.
Replace inline anonymous object types in session.sql.ts, revert.ts,
and session.ts with the new schema. Cast SessionPrompt input in
remote-sender to satisfy narrowed PromptInput type, guard against
null in permission normalizeInput, and switch remaining bare .parse
calls to .zod.parse for Effect Schema compatibility.

Include unit tests verifying SummaryFileDiff field exclusion and
round-trip parsing behavior.
Re-run code generation to emit the SnapshotSummaryFileDiff named type
and replace inline anonymous diff objects across Session,
SyncEventSessionUpdated, and GlobalSession. Alphabetically sort
dependency entries in the opencode package.json and add previously
appended Kilo-specific packages into their correct positions.
…cache paths

Remove mistral from the provider reasoning variant detection list
and update bare repo test assertions to use "kilo" cache directory
naming instead of "opencode".
Skip "glob tool keeps instance context during prompt runs" and
"prompt submitted during an active run" tests tracked in #9958.
Replace "build" agent assertion with "code" to align with Kilo
agent naming conventions.
Replace `$PWD` with `$(pwd -P)` when capturing the working directory
in zsh and bash login shell invocations. When the child shell inherits
a stale PWD environment variable — common in CI where the spawner's
cwd differs from the session directory — `$PWD` can point to the wrong
location, causing `cd "$__oc_cwd"` to silently escape the intended
directory. `pwd -P` queries the kernel via getcwd(), ensuring the
correct physical path is always used.

The corresponding test is updated with richer cwd diagnostics to
validate the fix and surface root-cause information on failure.
… of capturing it

Replace the `pwd -P` approach with an explicit positional argument (`$1`)
for zsh and bash login shell invocations. Login shells source profile
scripts (`/etc/profile`, `~/.profile`, `~/.bashrc`) before the `-c`
script executes, and those scripts may change the working directory
(e.g. nvm, direnv), making both `$PWD` and `pwd -P` unreliable at
script evaluation time. By passing `ctx.directory` as `$1` and falling
back to `$PWD` when unset, the shell script always receives the correct
session directory regardless of profile side effects.

Simplify the corresponding integration test by removing verbose CI
debug probes and replacing them with a straightforward `pwd` assertion.
…ling

Remove explanatory block comments and intermediate variable assignment
(`__oc_cwd`) that were left over from the previous iteration. Use
`cd -- "$1"` directly instead of the two-step capture-then-cd pattern,
and rename the argv[0] placeholder from `_opencode` to `opencode`.
const initialMeta = meta[0].value
const initialIcon = icon[0].value

const pathQuery = useQuery(() => loadPathQuery(directory))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: This query stays permanently skipped

loadPathQuery() only provides a queryFn when sdk and transform are passed. Calling it with just directory leaves the query on skipToken, so child stores created through globalSync.child(..., { bootstrap: false }) never populate store.path and keep returning the empty fallback path object.

const { Database, JsonMigration } = await import("virtual:opencode-server")
await JsonMigration.run(drizzle({ client: Database.Client().$client }), {
progress: (event: { current: number; total: number }) => {
const percent = Math.round(event.current / event.total) * 100
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Migration percentage only reports 0 or 100

Math.round(event.current / event.total) * 100 rounds before scaling, so most progress updates collapse to 0 until the ratio crosses 0.5. The loading window will look stuck for long migrations.

Suggested change
const percent = Math.round(event.current / event.total) * 100
const percent = Math.round((event.current / event.total) * 100)

},
})

win.webContents.session.webRequest.onBeforeSendHeaders((details, callback) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: These interceptors accumulate on the shared session

win.webContents.session is the default Electron session, not a window-scoped object. Every new window adds another pair of webRequest handlers here, and nothing removes them when the window closes, so header rewriting stacks up across windows and persists for the lifetime of the app.

await waitForHealth(port)
},
async remove(_config) {},
target(_config) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Multiple debug workspaces will point at the last created port

target() ignores the workspace config and reads a module-global PORT that is overwritten on every create(). If more than one debug workspace exists in the same process, older workspaces can start resolving to the newest workspace's server instead of their own.

log.info("downloading ripgrep", { url })
yield* fs.ensureDir(Global.Path.bin).pipe(Effect.orDie)

const bytes = yield* HttpClientRequest.get(url).pipe(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: This executes a downloaded binary without verifying the artifact

This branch now fetches a ripgrep release archive from GitHub, extracts it, and installs the executable into Global.Path.bin, but there is no checksum or signature validation before it is trusted. A compromised proxy, cache, or download path would turn this into arbitrary code execution inside the CLI.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Apr 25, 2026

Code Review Summary

Status: 5 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 5
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
packages/app/src/context/global-sync/child-store.ts 161 loadPathQuery(directory) is called without the sdk/transform needed to provide a queryFn, so child stores created without bootstrap keep an empty path.
packages/desktop-electron/src/main/index.ts 164 Migration progress rounds before multiplying, so the loading UI reports only 0 or 100 for most of the run.
packages/desktop-electron/src/main/windows.ts 103 webRequest interceptors are attached to the shared Electron session for every new window and are never removed, creating stacked handlers and a session-wide leak.
packages/opencode/src/control-plane/dev/debug-workspace-plugin.ts 62 target() reads a module-global PORT, so multiple debug workspaces in one process can resolve to the wrong server.
packages/opencode/src/file/ripgrep.ts 313 The fallback path downloads and executes a ripgrep binary without verifying the artifact checksum or signature.
Other Observations (not in diff)

No new issues were introduced in the incremental diff for the workflow and test changes below. The five warnings above are carried forward from unchanged files that remain unresolved.

Files Reviewed (5 files)
  • .github/workflows/test.yml - no new issues in the incremental diff
  • packages/opencode/test/file/index.test.ts - no new issues in the incremental diff
  • packages/opencode/test/file/ripgrep.test.ts - no new issues in the incremental diff
  • packages/opencode/test/tool/glob.test.ts - no new issues in the incremental diff
  • packages/opencode/test/tool/skill.test.ts - no new issues in the incremental diff

Reviewed by gpt-5.4-20260305 · 929,038 tokens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.