diff --git a/.github/releases/v1.0.39.md b/.github/releases/v1.0.39.md new file mode 100644 index 0000000000..eaff29d49f --- /dev/null +++ b/.github/releases/v1.0.39.md @@ -0,0 +1,48 @@ +## opencode {VERSION} + +{Prerelease/Stable} release from `{branch}` branch. A follow-up hardening train closing the v1.0.38 review leftovers: a credential-leak dependency fix, the platform group-kill primitive converged to a single source, and two test-robustness deliveries (a load-tolerant stall bound, real-component cleanup coverage), synced from dev back to main. + +--- + +### ๐Ÿ—๏ธ Architecture / Refactor + +- **Platform group-kill converged into one primitive, #515**: the win32 `taskkill /T /F` versus negative-pid `process.kill` branching existed as diverging copies in `util/process.stop` and the hook `execShell` kill group, with different error handling. `killGroupPid` is now the single source (POSIX signals the group; win32 awaits the taskkill exit code and throws, leaving fallback and logging to callers), reused by both call sites with zero behavior change: same signals, ordering, and grace constants, and the #500 process-group regression suite with real grandchildren passes. The published SDK keeps its documented synchronous copy (#503). + +--- + +### โš™๏ธ CI / Engineering + +- **Memory stall assertion made load-tolerant, #512**: the dead-connection test bounded a 40ms connectTimeout with a 200ms wall-clock assertion and flaked at 288ms on a loaded linux runner (raw setTimeout; TestClock cannot apply). The bound is now 2000ms, still separating fail-fast from hang, with the Stalled error-class assertion as the semantic check. +- **TUI route cleanup pinned against the real components, #514**: the #502 seam test mirrored the production subscription shape, so removing an onCleanup wrapper from the real routes left it green. The real Session and Prompt components now mount inside the production provider stack, and the SDK event bus must return to its baseline listener count after unmount; both wrappers were negative-verified (removal turns the test red). + +--- + +### ๐Ÿ“ฆ Dependencies / Tooling + +- **mysql2 3.14.4 to 3.22.0, #513**: closes the open high-severity dependabot advisory (alert 110) where an auth-plugin downgrade to mysql_clear_password leaks plaintext credentials. + +--- + +### ๐Ÿงช Test Summary + +``` +unit tests (linux): 4447 tests, 4423 pass, 0 fail (364 files) +tui suites: 48 pass, 0 fail in test/cli/tui (incl. 4 real-component cleanup tests) +targeted suites: memory + process + hook 215 pass, 0 fail (linux and macos) +typecheck: 29/29 packages green +lint: 4839 warnings, 0 errors (ratchet budget 4850) +``` + +--- + +### ๐Ÿ” Verification + +- Negative verification for #514: removing either production onCleanup wrapper turns the new real-component test red (Expected 0, Received 1); restoring turns it green, with an empty diff against production files afterwards. +- Zero-behavior-change argument for #515 walked per branch in the delivery commit: stop's win32 fallback path (taskkill failure to single-process kill) is preserved through the throw/catch reshape. +- The companion review finding on the workflow worker_type catalog's native-primary filter was adjudicated intentional (catalog lists spawn-resolvable worker shapes; native build/plan appear only because block defaults compile to them) and recorded on #507 without code change. +- Issues #512-#515 were closed manually with merge evidence: GitHub fires closing references only on PRs targeting the default branch, and this delivery merged through dev (PR #516) first. +- specgit finish exited 0 (accepted) for the dev delivery; this sync PR carries the same content to main under its own verdict. + +--- + +**Full changelog:** [`{previous_tag}`...`{current_tag}`](https://github.com/LeXwDeX/OpenCode-GraphAgent/compare/{previous_tag}...{current_tag}) diff --git a/.specgit.yaml b/.specgit.yaml index 106c10be2f..5c6fea502b 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,11 +1,11 @@ version: 1 -delivery: release-notes-v1-0-38 +delivery: sync-v1-0-39 context: kind: branch - branch: docs/510-release-notes-v1-0-38 + branch: chore/517-sync-v1-0-39 issues: - - 510 + - 517 issueKinds: - - issue: 510 - kind: kind::docs -pr: 511 + - issue: 517 + kind: kind::chore +pr: 518 diff --git a/bun.lock b/bun.lock index 7f4ae59fdb..f67c324ff2 100644 --- a/bun.lock +++ b/bun.lock @@ -193,7 +193,7 @@ "@types/node": "catalog:", "@typescript/native-preview": "catalog:", "drizzle-kit": "catalog:", - "mysql2": "3.14.4", + "mysql2": "3.22.0", "typescript": "catalog:", }, }, @@ -4487,7 +4487,7 @@ "mustache": ["mustache@4.2.0", "", { "bin": { "mustache": "bin/mustache" } }, "sha512-71ippSywq5Yb7/tVYyGbkBggbU8H3u5Rz56fH60jGFgr8uHwxs+aSKeqmluIVzM0m0kB7xQjKS6qPfd0b2ZoqQ=="], - "mysql2": ["mysql2@3.14.4", "", { "dependencies": { "aws-ssl-profiles": "^1.1.1", "denque": "^2.1.0", "generate-function": "^2.3.1", "iconv-lite": "^0.7.0", "long": "^5.2.1", "lru.min": "^1.0.0", "named-placeholders": "^1.1.3", "seq-queue": "^0.0.5", "sqlstring": "^2.3.2" } }, "sha512-Cs/jx3WZPNrYHVz+Iunp9ziahaG5uFMvD2R8Zlmc194AqXNxt9HBNu7ZsPYrUtmJsF0egETCWIdMIYAwOGjL1w=="], + "mysql2": ["mysql2@3.22.0", "", { "dependencies": { "aws-ssl-profiles": "^1.1.2", "denque": "^2.1.0", "generate-function": "^2.3.1", "iconv-lite": "^0.7.2", "long": "^5.3.2", "lru.min": "^1.1.4", "named-placeholders": "^1.1.6", "sql-escaper": "^1.3.3" }, "peerDependencies": { "@types/node": ">= 8" } }, "sha512-4jaJYBObj7FhD3lnZhqX1yDMuZN4mQNz+IolDySDXT7fbozMBpeGQNcuWXKUqo4ahkAEfkjUHPjnwuDI0/6VKw=="], "mz": ["mz@2.7.0", "", { "dependencies": { "any-promise": "^1.0.0", "object-assign": "^4.0.1", "thenify-all": "^1.0.0" } }, "sha512-z81GNO7nnYMEhrGh9LeymoE4+Yr0Wn5McHIZMK5cfQCl+NDX08sCZgUc9/6MHni9IWuFLm1Z3HTCXu2z9fN62Q=="], @@ -5019,8 +5019,6 @@ "send": ["send@0.19.2", "", { "dependencies": { "debug": "2.6.9", "depd": "2.0.0", "destroy": "1.2.0", "encodeurl": "~2.0.0", "escape-html": "~1.0.3", "etag": "~1.8.1", "fresh": "~0.5.2", "http-errors": "~2.0.1", "mime": "1.6.0", "ms": "2.1.3", "on-finished": "~2.4.1", "range-parser": "~1.2.1", "statuses": "~2.0.2" } }, "sha512-VMbMxbDeehAxpOtWJXlcUS5E8iXh6QmN+BkRX1GARS3wRaXEEgzCcB10gTQazO42tpNIya8xIyNx8fll1OFPrg=="], - "seq-queue": ["seq-queue@0.0.5", "", {}, "sha512-hr3Wtp/GZIc/6DAGPDcV4/9WoZhjrkXsi5B/07QgX8tsdc6ilr7BFM6PM6rbdAX1kFSDYeZGLipIZZKyQP0O5Q=="], - "serialize-error": ["serialize-error@7.0.1", "", { "dependencies": { "type-fest": "^0.13.1" } }, "sha512-8I8TjW5KMOKsZQTvoxjuSIa7foAwPWGOts+6o7sgjz41/qMD9VQHEDxi6PBvK2l0MXUmqZyNpUK+T2tQaaElvw=="], "seroval": ["seroval@1.3.2", "", {}, "sha512-RbcPH1n5cfwKrru7v7+zrZvjLurgHhGyso3HTyGtRivGWgYjbOmGuivCQaORNELjNONoK35nj28EoWul9sb1zQ=="], @@ -5125,7 +5123,7 @@ "sprintf-js": ["sprintf-js@1.0.3", "", {}, "sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g=="], - "sqlstring": ["sqlstring@2.3.3", "", {}, "sha512-qC9iz2FlN7DQl3+wjwn3802RTyjCx7sDvfQEXchwa6CWOx07/WVfh91gBmQ9fahw8snwGEWU3xGzOt4tFyHLxg=="], + "sql-escaper": ["sql-escaper@1.5.1", "", {}, "sha512-4toX5E1fQbBrpfXidaHnF0669nkAdETeIPTs2SUjxxD7RRIs9ICG4gtpmfc68JCEKehsdwLFqBu9VlQqZ1P1gg=="], "srvx": ["srvx@0.9.8", "", { "bin": { "srvx": "bin/srvx.mjs" } }, "sha512-RZaxTKJEE/14HYn8COLuUOJAt0U55N9l1Xf6jj+T0GoA01EUH1Xz5JtSUOI+EHn+AEgPCVn7gk6jHJffrr06fQ=="], diff --git a/packages/console/core/package.json b/packages/console/core/package.json index bc7c31cc0b..05df1d3c88 100644 --- a/packages/console/core/package.json +++ b/packages/console/core/package.json @@ -45,7 +45,7 @@ "@types/bun": "catalog:", "@types/node": "catalog:", "drizzle-kit": "catalog:", - "mysql2": "3.14.4", + "mysql2": "3.22.0", "typescript": "catalog:", "@typescript/native-preview": "catalog:" } diff --git a/packages/opencode/src/hook/settings.ts b/packages/opencode/src/hook/settings.ts index cf5d6bc3b9..c9851b2575 100644 --- a/packages/opencode/src/hook/settings.ts +++ b/packages/opencode/src/hook/settings.ts @@ -51,6 +51,7 @@ import { generateObject, generateText, type ModelMessage } from "ai" import { FSUtil } from "@opencode-ai/core/fs-util" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" import * as Log from "@/util/log" +import { Process } from "@/util/process" import { Global } from "@opencode-ai/core/global" import { InstanceState } from "@/effect/instance-state" import { MCP } from "@/mcp" @@ -1143,18 +1144,9 @@ function execShell( const killGroup = () => { if (killSent || child.pid === undefined) return killSent = true - if (process.platform === "win32") { - spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore", windowsHide: true }).on( - "error", - (err) => log.warn("hook taskkill failed", { command, error: err.message }), - ) - return - } - try { - process.kill(-child.pid, "SIGKILL") - } catch (err) { + void Process.killGroupPid(child.pid, "SIGKILL").catch((err) => { log.warn("hook process-group kill failed", { command, error: String(err) }) - } + }) } const afterKill = () => { killGroup() diff --git a/packages/opencode/src/util/process.ts b/packages/opencode/src/util/process.ts index b24538f7de..94d82cee87 100644 --- a/packages/opencode/src/util/process.ts +++ b/packages/opencode/src/util/process.ts @@ -150,6 +150,20 @@ export async function run(cmd: string[], opts: RunOptions = {}): Promise export const STOP_TERM_GRACE_MS = 3_000 export const STOP_KILL_GRACE_MS = 2_000 +// Platform group-kill primitive: POSIX signals the process group led by `pid` +// (the child must be a detached group leader); win32 has no group semantics, +// so `taskkill /T /F` tree-kills instead and `signal` is ignored. Resolves +// once the kill is delivered โ€” on win32 that means awaiting the taskkill exit +// code โ€” and throws when delivery fails, leaving fallback and logging to the +// caller. +export async function killGroupPid(pid: number, signal: NodeJS.Signals = "SIGKILL") { + if (process.platform !== "win32") { + process.kill(-pid, signal) + return + } + await run(["taskkill", "/pid", String(pid), "/T", "/F"]) +} + // Duplicated in `packages/sdk/js/src/process.ts` because the SDK cannot import // `opencode` without creating a cycle. Keep both copies in sync. export async function stop(proc: ChildProcess) { @@ -163,12 +177,11 @@ export async function stop(proc: ChildProcess) { return } - const out = await run(["taskkill", "/pid", String(proc.pid), "/T", "/F"], { - nothrow: true, - }) - - if (out.code === 0) return - proc.kill() + try { + await killGroupPid(proc.pid) + } catch { + proc.kill() + } } function exitedWithin(proc: ChildProcess, timeoutMs: number) { diff --git a/packages/opencode/test/memory/memory.test.ts b/packages/opencode/test/memory/memory.test.ts index f6e85eb5d3..cd0ea57fcd 100644 --- a/packages/opencode/test/memory/memory.test.ts +++ b/packages/opencode/test/memory/memory.test.ts @@ -1716,7 +1716,11 @@ describe("memory hidden model", () => { catch: (cause) => cause, }).pipe(Effect.flip) expect(error instanceof MemoryModel.Stalled).toBe(true) - expect(Date.now() - started).toBeLessThan(200) + // Fail-fast bound, not a scheduler bound: drainWithLiveness arms raw + // setTimeout (no TestClock), and a loaded linux runner was observed + // firing the 40ms connectTimeout at 288ms. 2000ms still separates + // fail-fast from hang (a hang trips the test timeout instead). + expect(Date.now() - started).toBeLessThan(2000) }), ) diff --git a/packages/tui/test/cli/tui/event-cleanup.test.tsx b/packages/tui/test/cli/tui/event-cleanup.test.tsx index bb4420d08e..6522926b04 100644 --- a/packages/tui/test/cli/tui/event-cleanup.test.tsx +++ b/packages/tui/test/cli/tui/event-cleanup.test.tsx @@ -1,12 +1,39 @@ /** @jsxImportSource @opentui/solid */ import { describe, expect, test } from "bun:test" -import { testRender } from "@opentui/solid" +import { testRender, useRenderer } from "@opentui/solid" +import { createDefaultOpenTuiKeymap } from "@opentui/keymap/opentui" import type { Event, GlobalEvent } from "@opencode-ai/sdk/v2" import { createSignal, onCleanup, onMount, Show } from "solid-js" -import { SDKProvider } from "../../../src/context/sdk" +import { Prompt } from "../../../src/component/prompt" +import { TuiConfigProvider } from "../../../src/config" +import { ArgsProvider } from "../../../src/context/args" +import { ClipboardProvider } from "../../../src/context/clipboard" +import { DataProvider } from "../../../src/context/data" +import { EditorContextProvider } from "../../../src/context/editor" +import { EpilogueProvider } from "../../../src/context/epilogue" import { useEvent } from "../../../src/context/event" -import { createEventSource, createFetch, directory } from "../../fixture/tui-sdk" +import { ExitProvider } from "../../../src/context/exit" +import { KVProvider } from "../../../src/context/kv" +import { LocalProvider } from "../../../src/context/local" +import { LocationProvider } from "../../../src/context/location" +import { ProjectProvider } from "../../../src/context/project" +import { PromptRefProvider } from "../../../src/context/prompt" +import { RouteProvider } from "../../../src/context/route" +import { SDKProvider, useSDK } from "../../../src/context/sdk" +import { SyncProvider, useSync } from "../../../src/context/sync" +import { ThemeProvider } from "../../../src/context/theme" +import { OpencodeKeymapProvider, registerOpencodeKeymap } from "../../../src/keymap" +import { createPluginRuntime, PluginRuntimeProvider } from "../../../src/plugin/runtime" +import { FrecencyProvider } from "../../../src/prompt/frecency" +import { PromptHistoryProvider } from "../../../src/prompt/history" +import { PromptStashProvider } from "../../../src/prompt/stash" +import { Session } from "../../../src/routes/session" +import { DialogProvider } from "../../../src/ui/dialog" +import { ToastProvider } from "../../../src/ui/toast" +import { tmpdir } from "../../fixture/fixture" import { TestTuiContexts } from "../../fixture/tui-environment" +import { createTuiResolvedConfig } from "../../fixture/tui-runtime" +import { createEventSource, createFetch, directory, json } from "../../fixture/tui-sdk" // Route components (routes/session/index.tsx, component/prompt/index.tsx) // subscribe to app-level events via `onCleanup(event.on(...))` so the @@ -132,3 +159,213 @@ describe("event.on cleanup", () => { } }) }) + +// The seam tests above pin the event-context contract with a probe that +// mirrors the production subscription shape. The tests below mount the REAL +// route components (routes/session, component/prompt) that #502 wrapped in +// `onCleanup(event.on(...))` and count listeners on the real SDK event +// emitter: mounting must add listeners, unmounting must return the bus to +// the pre-mount baseline, and mount/unmount cycles must not accumulate. +// Removing any of the production `onCleanup` wrappers leaves a listener +// behind and turns these red. + +const routeSessionID = "ses_real_route" + +const routeSession = { + id: routeSessionID, + title: "event cleanup", + time: { created: 0, updated: 0 }, + version: "1.17.11", + directory, +} + +function routeFetch(url: URL) { + if (url.pathname === `/session/${routeSessionID}`) return json(routeSession) + if (url.pathname.startsWith(`/session/${routeSessionID}/`)) return json([]) + return undefined +} + +// Counting bridge installed on the real bus seam: every `useEvent()` +// registration (Session, Prompt, and anything they mount) flows through +// `sdk.event.on("event", ...)`, so wrapping that method yields the net +// listener count without touching component internals. Subscriptions the +// providers install before the bridge are app-lifetime constants and stay +// outside the count; the baseline is captured after the full provider stack +// reports ready. +function BusCounter(props: { onReady: (counter: () => number) => void }) { + const sdk = useSDK() + let count = 0 + // oxlint-disable-next-line typescript-eslint/unbound-method -- the reference IS the restoration point for onCleanup; every invocation goes through bind with an explicit receiver. + const original = sdk.event.on + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- sdk.event.on is overloaded; the counting bridge needs one widened call signature and bind preserves the receiver. + const register = original.bind(sdk.event) as (type: string, handler: unknown) => () => void + const counting = ((type: string, handler: unknown) => { + const off = register(type, handler) + if (type !== "event") return off + count += 1 + let released = false + return () => { + if (released) return + released = true + count -= 1 + off() + } + }) as typeof original + sdk.event.on = counting + onCleanup(() => { + sdk.event.on = original + }) + props.onReady(() => count) + return +} + +function SyncProbe(props: { onSync: (sync: ReturnType) => void }) { + props.onSync(useSync()) + return +} + +// Mirrors the production provider stack from src/app.tsx (same order, test +// fixtures for config/keymap/paths/fetch). The route component is gated +// behind a signal so the baseline can be captured before it mounts. +async function mountRoute(view: "session" | "prompt") { + const tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + const events = createEventSource() + const calls = createFetch(routeFetch) + const [mounted, setMounted] = createSignal(false) + let sync!: ReturnType + let counter!: () => number + let resolveReady!: () => void + const ready = new Promise((resolve) => { + resolveReady = resolve + }) + + function Harness() { + const renderer = useRenderer() + const keymap = createDefaultOpenTuiKeymap(renderer) + const config = createTuiResolvedConfig() + onCleanup(registerOpencodeKeymap(keymap, renderer, config)) + + return ( + + {}}> + {}}> + + + + + + + + + + + + + + + + + + + + + + { + counter = count + resolveReady() + }} + /> + (sync = value)} /> + + {view === "session" ? ( + + ) : ( + + )} + + + + + + + + + + + + + + + + + + + + + + + + + + ) + } + + const app = await testRender(() => , { width: 80, height: 24 }) + await ready + await wait(() => sync.status === "complete") + return { + baseline: counter(), + counter, + mount: () => setMounted(true), + unmount: () => setMounted(false), + async dispose() { + app.renderer.destroy() + await tmp[Symbol.asyncDispose]() + }, + } +} + +describe("real route component event cleanup", () => { + test("Session route returns the event bus to its baseline listener count after unmount", async () => { + const route = await mountRoute("session") + + try { + route.mount() + await wait(() => route.counter() > route.baseline) + route.unmount() + expect(route.counter()).toBe(route.baseline) + + route.mount() + await wait(() => route.counter() > route.baseline) + route.unmount() + expect(route.counter()).toBe(route.baseline) + } finally { + await route.dispose() + } + }) + + test("Prompt returns the event bus to its baseline listener count after unmount", async () => { + const route = await mountRoute("prompt") + + try { + route.mount() + await wait(() => route.counter() > route.baseline) + route.unmount() + expect(route.counter()).toBe(route.baseline) + + route.mount() + await wait(() => route.counter() > route.baseline) + route.unmount() + expect(route.counter()).toBe(route.baseline) + } finally { + await route.dispose() + } + }) +})