sync: upstream v1.18.4 (d36a2d898 on dev)#116
Open
Alezander9 wants to merge 922 commits into
Open
Conversation
Co-authored-by: Kit Langton <kit.langton@gmail.com>
Co-authored-by: Luke Parker <10430890+Hona@users.noreply.github.com>
Co-authored-by: Brendan Allan <git@brendonovich.dev>
Co-authored-by: Brendan Allan <git@brendonovich.dev>
…#36066) Co-authored-by: Aarav Sareen <96787824+arvsrn@users.noreply.github.com>
Co-authored-by: Jay <53023+jayair@users.noreply.github.com>
Co-authored-by: Aiden Cline <rekram1-node@users.noreply.github.com>
Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>
Co-authored-by: Aiden Cline <rekram1-node@users.noreply.github.com>
…… (#37696) Co-authored-by: liqiping <liqiping@msh.team> Co-authored-by: Aiden Cline <aidenpcline@gmail.com>
Co-authored-by: Aiden Cline <rekram1-node@users.noreply.github.com>
Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>
Co-authored-by: Jay <53023+jayair@users.noreply.github.com>
Co-authored-by: Luke Parker <10430890+Hona@users.noreply.github.com>
Co-authored-by: Luke Parker <10430890+Hona@users.noreply.github.com>
Includes the physical-pixel rendering and native Ghostty sprite work from anomalyco/ghostty-web#3.
Co-authored-by: Brendan Allan <14191578+Brendonovich@users.noreply.github.com>
Co-authored-by: Brendan Allan <14191578+Brendonovich@users.noreply.github.com>
Co-authored-by: Brendan Allan <14191578+Brendonovich@users.noreply.github.com>
Requested by: @thdxr (dax via Slack)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 1858 files
Not reviewed (too large): packages/opencode/test/tool/fixtures/models-api.json (~188,953 lines), packages/stats/app/src/routes/index.css (~6,002 lines), packages/console/core/migrations/20260626161712_oval_callisto/snapshot.json (~3,093 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/app/e2e/tsconfig.json">
<violation number="1" location="packages/app/e2e/tsconfig.json:9">
P1: The new include list drops ~75+ existing test files from type-checking. The old `./**/*.ts` wildcard ensured all e2e `.ts` files were compiled; the new explicit list only covers about 17 file paths, silently excluding most performance, regression, and smoke test files from `tsc --noEmit`. Either restore `./**/*.ts` or add explicit patterns that cover the missing directories.</violation>
</file>
<file name="packages/app/e2e/utils/visual-stability/capture.ts">
<violation number="1" location="packages/app/e2e/utils/visual-stability/capture.ts:35">
P2: Capture-loop errors are silently swallowed — a CDP disconnect, screenshot failure, or any thrown error in the while-loop body goes completely invisible, making debugging of flaky or broken captures needlessly hard. Log the error (e.g. console.warn) in the catch block so capture failures are observable.</violation>
</file>
<file name="packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts">
<violation number="1" location="packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts:25">
P2: When no stable three-sample window exists, `stable` is `-1`, so `stable + 2` points at the second sample and falsely reports its timestamp as `stableObservedMs`. Guard the index before reading the sample so unsuccessful classification remains `null`.</violation>
</file>
<file name="infra/stats.ts">
<violation number="1" location="infra/stats.ts:89">
P1: Adding `ignoreChanges: ["metadata"]` alongside new schema fields (model, model_tier, user_id) means Pulumi will skip applying metadata/schema changes on subsequent updates for already-deployed tables — so these new columns probably won't actually be created where the table already exists. If the intent is only to avoid the risky delete-before-replace behavior in production, consider a narrower approach (e.g., scoping ignoreChanges only when the table already has the desired schema, or handling schema evolution separately) so new columns still get applied.</violation>
</file>
<file name="packages/codemode/src/stdlib/number.ts">
<violation number="1" location="packages/codemode/src/stdlib/number.ts:29">
P2: The manually-thrown radix-range error for `Number.toString` doesn't call `.as("RangeError")`, unlike the equivalent range-check pattern used elsewhere in stdlib (e.g. `string.ts`'s `normalize`, `url.ts`, `json.ts`). Without it, `errorName` defaults to `"Error"`, so `catch (e) { e.name }` in a CodeMode program will see `"Error"` instead of `"RangeError"`, diverging from real JS semantics and from the sibling stdlib convention.</violation>
</file>
<file name="packages/cli/src/commands/handlers/serve.ts">
<violation number="1" location="packages/cli/src/commands/handlers/serve.ts:32">
P2: This linear port scan retries on any bind failure (not just EADDRINUSE), so a non-port-conflict error (e.g. permission or hostname resolution issue) will now trigger up to ~61,439 sequential bind attempts from 4096 to 65535 before the real error surfaces, instead of failing fast or falling back to an OS-assigned port as before. Consider limiting the retry to address-in-use errors and/or falling back to port 0 once a reasonable range is exhausted.</violation>
</file>
<file name="packages/app/src/pages/session/composer/session-composer-region-controller.ts">
<violation number="1" location="packages/app/src/pages/session/composer/session-composer-region-controller.ts:64">
P2: The dock 'ready' flag is never reset when `sessionKey` changes, but the same controller instance persists across session switches (the enclosing `<Show>` in session.tsx isn't keyed on session id). As a result the intended entrance delay/animation for the composer dock only fires for the very first session and is silently skipped on every subsequent session switch.</violation>
</file>
<file name="packages/client/tsconfig.json">
<violation number="1" location="packages/client/tsconfig.json:1">
P2: packages/client/tsconfig.json is called out in AGENTS.md as a Red/Yellow-zone file that should only be touched with a documented justification in memory/browsercode/EXCEPTIONS.md, but this PR creates it with no such entry. Please add the required justification note or confirm this file is exempt before merging.</violation>
</file>
<file name="packages/app/src/pages/session/composer/session-todo-dock.tsx">
<violation number="1" location="packages/app/src/pages/session/composer/session-todo-dock.tsx:90">
P2: Using `Math.max(height, el.scrollHeight)` makes `store.height` monotonically increasing, so once the todo list grows tall it will never shrink again even after todos are completed or removed, leaving extra empty space in the dock. If shrink-avoidance during animation is intended, consider resetting/recomputing height when `props.todos` changes rather than always taking the max of all-time values.</violation>
</file>
<file name="packages/app/src/pages/session/timeline/model.ts">
<violation number="1" location="packages/app/src/pages/session/timeline/model.ts:122">
P2: The removed loop (and its explanatory comment) handled pages that only contain assistant messages or user turns hidden by a revert boundary by continuing to fetch until visible progress was made or there was nothing left to load. The new single-shot implementation always reports `done=true` after one fetch, so a history page with no visible user messages will stop the load sequence immediately instead of continuing, which can make 'load older' UX require extra manual scroll/trigger to actually reveal new visible messages.</violation>
</file>
<file name="packages/cli/script/build.ts">
<violation number="1" location="packages/cli/script/build.ts:66">
P2: The tree-sitter parser-worker bundling (parserWorker resolution, extra entrypoint, and OTUI_TREE_SITTER_WORKER_PATH define) was removed from this build script without any replacement, unlike packages/opencode/script/build.ts which now embeds the worker text via `files`/entrypoints and still sets OTUI_TREE_SITTER_WORKER_PATH. If the lildax binary's @opentui/core usage depends on tree-sitter highlighting, the compiled binary will have no worker path configured and highlighting could silently fail at runtime; please confirm this package genuinely doesn't need the worker, or port over the embedded-text approach used in packages/opencode/script/build.ts.</violation>
</file>
<file name="packages/app/src/hooks/use-providers.ts">
<violation number="1" location="packages/app/src/hooks/use-providers.ts:27">
P2: Passing an explicit `directory` accessor now returns an empty provider catalog whenever that project's store isn't `provider_ready` (or the directory is momentarily undefined), instead of falling back to the global provider list as before. This can cause UI flicker (e.g. empty model list in dialog-select-model-unpaid, zero context providers) for directory-scoped callers during initial load — worth confirming this loss of fallback is intentional.</violation>
</file>
<file name="packages/app/src/components/prompt-input/submission-state.ts">
<violation number="1" location="packages/app/src/components/prompt-input/submission-state.ts:19">
P2: clear() resets the prompt text/cursor on `initial` and `target` but never clears their context items, so after retargeting to a newly created session the original draft's file-context items are left attached and will resurface next time that composer scope is reused. Consider explicitly clearing `initial.context`/`target.context` (similar to the existing `clearContext` helper in submit.ts) inside `clear()`.</violation>
</file>
<file name="packages/app/src/context/models.tsx">
<violation number="1" location="packages/app/src/context/models.tsx:164">
P2: Converting the previously synchronous `recent.list` memo into an async `createResource` (using an async function as the resource "source" with an identity fetcher) means `models.recent.list()` can now throw if `ready.promise` rejects, and adds a microtask delay after `push()` before the list reflects the update — neither was possible with the old `createMemo(() => store.recent)`. Since `ready()` (from `persisted`) already exposes a non-throwing boolean, a memo like `createMemo(() => (ready() ? store.recent : []))` would give the same "wait until ready" behavior without the added error-throw risk and complexity.</violation>
</file>
<file name="packages/app/src/context/command.tsx">
<violation number="1" location="packages/app/src/context/command.tsx:243">
P3: `formatKeybindKeys` is added but never called anywhere in the app; it's a byte-for-byte duplicate of `formatKeybindParts`. Either wire it up where KeybindV2 needs it or drop it to avoid dead exported code.</violation>
<violation number="2" location="packages/app/src/context/command.tsx:414">
P2: Switching the document keydown listener to `{ capture: true }` plus adding `stopPropagation()` means any registered global keybind now pre-empts embedded components (e.g. terminal) before they ever see the keystroke, not just after they've had a chance to handle it as before. Commands without a `when` guard (like `input.focus` on `ctrl+l`, a common shell clear-screen shortcut) could now silently swallow keystrokes that used to reach the terminal/editor first — worth double-checking terminal shortcut behavior for keybinds outside `EDITABLE_KEYBIND_IDS`/`when`-guarded commands.</violation>
</file>
<file name="packages/app/src/components/titlebar.css">
<violation number="1" location="packages/app/src/components/titlebar.css:22">
P2: These selectors reference `[data-slot="titlebar-tab-close"]` / `[data-slot="titlebar-tab-close-fade"]`, but the tab close button in `titlebar-tab-nav.tsx`/`titlebar-tab-nav.css` actually uses `data-slot="tab-close"`. As written, these rules never match anything, so the focus-visible repositioning/fade override is dead code.</violation>
</file>
<file name="packages/app/src/pages/session/composer/prompt-model-selection.ts">
<violation number="1" location="packages/app/src/pages/session/composer/prompt-model-selection.ts:10">
P2: This new file exports `createPromptModelSelection`, but nothing in the repo imports or calls it — the composer still wires model selection through `useLocal()`/`local.tsx`. Either wire this new helper into its intended consumer or remove it to avoid shipping unused/duplicate model-selection logic (it largely re-implements the `model`/`variant` logic already in `context/local.tsx`).</violation>
</file>
<file name="packages/app/src/components/session/session-sortable-tab-v2.tsx">
<violation number="1" location="packages/app/src/components/session/session-sortable-tab-v2.tsx:39">
P2: SortableTabV2 doesn't apply any dragging-state styling (no `opacity-0`/`isDragSource` handling like the original `SortableTab` and other dnd-kit sortables in this codebase use). This means when a tab is dragged, both the floating drag preview and the original tab will be visible simultaneously, which is a visual regression from the v1 component.</violation>
</file>
<file name="packages/app/src/components/prompt-input/placeholder.ts">
<violation number="1" location="packages/app/src/components/prompt-input/placeholder.ts:19">
P2: New placeholder text is hardcoded in English rather than routed through the existing i18n `t()` function used by `promptPlaceholder`, so this string won't be localized for non-English users.</violation>
</file>
<file name="packages/app/src/pages/home-session-archive.ts">
<violation number="1" location="packages/app/src/pages/home-session-archive.ts:28">
P2: Chaining `.catch()` after `.then()` here also catches exceptions thrown inside the `.then()` callback (e.g. from `input.remove()`), so a failure in the UI-update/notify step would be reported through `onError` as if the archive request itself failed, even though it succeeded. Consider separating success-path errors from the update-request error, e.g. by wrapping `input.remove()`/`notifySessionTabsRemoved` in their own try/catch or awaiting update first and handling side effects outside the catch.</violation>
</file>
<file name="packages/app/src/pages/session/session-model-helpers.ts">
<violation number="1" location="packages/app/src/pages/session/session-model-helpers.ts:45">
P2: The equality check in `syncPromptModel` doesn't normalize `null` vs `undefined` for `variant` the way `restorePromptModel` does, so a persisted `variant: null` can be treated as different from the current `undefined` variant and cause a redundant prompt-model write on sync.</violation>
</file>
<file name="packages/app/src/pages/session/session-panel-layout.ts">
<violation number="1" location="packages/app/src/pages/session/session-panel-layout.ts:4">
P2: `stacked` doesn't account for `files`, even though `visible` does. When the file tree and terminal are both open (without review), the side panel and terminal panel will both stretch to fill space without the resize handle that appears for the review+terminal case — likely an unintended layout inconsistency for that combination.</violation>
</file>
<file name="packages/app/src/context/global-sync/event-reducer.ts">
<violation number="1" location="packages/app/src/context/global-sync/event-reducer.ts:123">
P2: The new `sessionContent`/`permission` gating and `reference.updated` handling change control flow for a wide set of events but aren't covered by event-reducer.test.ts; consider adding cases (gate on/off, permission override during trim, reference.updated dispatch) to prevent future regressions in this event routing.</violation>
</file>
<file name="packages/app/src/pages/layout/sidebar-project.tsx">
<violation number="1" location="packages/app/src/pages/layout/sidebar-project.tsx:307">
P2: isWorking now scans the global session_status map (all sessions across all projects) for each workspace directory via `serverSync().session.get(id)?.directory`, instead of reading the directory-scoped store. This causes the memo to recompute for every project tile on any session status change app-wide, and scales with total session count rather than per-directory session count, which could hurt sidebar responsiveness once many projects/sessions are open.</violation>
</file>
<file name="packages/app/src/components/edit-project.ts">
<violation number="1" location="packages/app/src/components/edit-project.ts:67">
P2: The `save` mutation has no `onError` handling, so if the project update request fails the dialog silently stays open with no feedback to the user about why saving failed. Consider surfacing the error (e.g., a toast or inline error message) similar to other save mutations in the app.</violation>
</file>
<file name="packages/app/src/components/ui/drawer.tsx">
<violation number="1" location="packages/app/src/components/ui/drawer.tsx:58">
P2: DrawerContent's positioning classes are hardcoded for the right side (`right-[6px] left-auto`, fixed width `w-[560px]`) with no `data-[side=...]` styling to react to `@corvu/drawer`'s `side` prop. If this component is ever reused with `side="left"`, `"top"`, or `"bottom"` (as the current only consumer, help-button.tsx, already passes `side="right"` explicitly, suggesting other sides are anticipated), the drawer will render off-screen or with the wrong layout with no compile-time warning.</violation>
</file>
<file name="packages/app/src/context/layout-tabs.ts">
<violation number="1" location="packages/app/src/context/layout-tabs.ts:16">
P2: previewSessionTab doesn't special-case the pinned "review"/"context" tab IDs the way openSessionTab and closeSessionTab do, so if it's ever invoked with those values it will insert "review" into `tabs.all`, breaking the invariant the other two functions rely on. Consider adding the same guard clauses here for consistency, or documenting that previewSessionTab must never be called with these reserved tab names.</violation>
</file>
<file name="packages/codemode/src/stdlib/json.ts">
<violation number="1" location="packages/codemode/src/stdlib/json.ts:16">
P2: The replacer-detection guard only catches `Array` and `CodeModeFunction`, but the sandbox also represents callables as `GlobalMethodReference`, `IntrinsicReference`, `PromiseMethodReference`, `CoercionFunction`, `UriFunction`, and `ErrorConstructorReference` (see runtime.ts's broader callable checks). Passing one of these as the replacer silently skips the 'replacers not supported' diagnostic instead of failing closed as intended.</violation>
</file>
<file name="packages/codemode/src/stdlib/string.ts">
<violation number="1" location="packages/codemode/src/stdlib/string.ts:46">
P2: `invokeStringStatic`'s `fromCodePoint` branch doesn't guard against native `RangeError` the way other stdlib functions (e.g. `String.normalize`, `URL` statics, `RegExp` construction) do, so an invalid code point produces a generic execution failure instead of a clear CodeMode diagnostic. Consider wrapping the `String.fromCodePoint(...codes)` call in try/catch and rethrowing as an `InterpreterRuntimeError` with `.as("RangeError")` for consistency.</violation>
</file>
<file name="packages/app/src/components/file-tree-v2-model.ts">
<violation number="1" location="packages/app/src/components/file-tree-v2-model.ts:32">
P2: If the input path list ever contains a path that is also a leaf file for one entry and a directory prefix for another (order-dependent), `buildFileTreeV2Model` can leave that node typed as "file" while still attaching children to it in the tree, causing those children to be silently dropped by `flattenFileTreeV2`. Consider promoting the node to "directory" whenever it's encountered as a non-leaf segment, regardless of whether it was already created.</violation>
</file>
<file name="packages/app/e2e/utils/visual-stability.ts">
<violation number="1" location="packages/app/e2e/utils/visual-stability.ts:24">
P3: `markVisualStability` breaks the naming pattern established by `startVisualStabilityProbe`/`stopVisualStabilityProbe`. It wraps `markVisualProbe` but drops the "Probe" suffix, making the API less predictable.</violation>
</file>
<file name="packages/app/src/pages/session/session-ownership.ts">
<violation number="1" location="packages/app/src/pages/session/session-ownership.ts:16">
P2: Both the returned `key()` function and the `capture()` result's `key` field share the name `key` but have different types (function vs string), which is an easy foot-gun for future callers who might call `owner.key()` (TypeError, not a function) or forget to call `sessionOwnership.key()`. Consider renaming one of them (e.g. `currentKey()` vs `capturedKey`) to avoid confusion.</violation>
</file>
<file name="packages/codemode/src/stdlib/value.ts">
<violation number="1" location="packages/codemode/src/stdlib/value.ts:60">
P2: When the coerced value is a sandbox value (Date/RegExp/Map/Set/URL/URLSearchParams), `parseInt` ignores any radix passed as the second argument, unlike the non-sandbox path a few lines below which validates and forwards it. This makes `parseInt(sandboxDate, 16)` silently behave as base-10/auto-detect instead of respecting the radix.</violation>
</file>
<file name="packages/client/README.md">
<violation number="1" location="packages/client/README.md:26">
P3: The usage example creates a session but discards the result, then uses an undefined `sessionID` variable in the following `prompt` call. Readers copying this snippet will hit a reference error; capture the created session and pull the ID from it.</violation>
</file>
<file name="packages/app/src/context/global.tsx">
<violation number="1" location="packages/app/src/context/global.tsx:131">
P3: This recentlyClosed computation (filter known worktrees via pathKey, slice to RECENTLY_CLOSED_DISPLAY_LIMIT, map through enrich) duplicates the identical logic already in layout.tsx's `projects.recentlyClosed`. Consider extracting a shared helper (e.g. in server.ts) so both contexts stay in sync if the filtering/limit logic changes later.</violation>
</file>
<file name="packages/app/src/components/command-tooltip-keybind.ts">
<violation number="1" location="packages/app/src/components/command-tooltip-keybind.ts:5">
P3: Both `reviewTooltipKeybind` and `newTabTooltipKeybind` accept a `_translate` callback that is never called, so any intended localization of keybind labels is silently dropped. Either wire the translate function into the returned keybind parts or drop the unused parameter to avoid misleading callers/tests into thinking translation is applied.</violation>
</file>
<file name="packages/app/src/components/model-tooltip.tsx">
<violation number="1" location="packages/app/src/components/model-tooltip.tsx:65">
P3: `name()` duplicates the tag-building logic already in `title()` just above it; consider extracting a shared `tagSuffix()` helper used by both to avoid the two implementations drifting apart later.</violation>
</file>
<file name="packages/app/src/components/prompt-input/transient-state.ts">
<violation number="1" location="packages/app/src/components/prompt-input/transient-state.ts:17">
P3: The reset defaults and the initial store defaults are duplicated in two places; consider extracting a shared `DEFAULTS` object (excluding `placeholder`) and spreading it in both `createStore` and `resetPromptInputTransientState` to avoid future drift when new fields are added.</violation>
</file>
<file name="packages/app/src/pages/layout-new.tsx">
<violation number="1" location="packages/app/src/pages/layout-new.tsx:48">
P3: The new layout drops the `VITE_DISABLE_DEBUG_BAR` env check that the legacy layout uses before rendering `DebugBar`, so setting that env var will no longer hide the debug bar when the new layout is active, creating inconsistent behavior between the two layouts.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| }, | ||
| "include": ["./**/*.ts"] | ||
| "include": [ | ||
| "./performance/timeline-stability/**/*.spec.ts", |
There was a problem hiding this comment.
P1: The new include list drops ~75+ existing test files from type-checking. The old ./**/*.ts wildcard ensured all e2e .ts files were compiled; the new explicit list only covers about 17 file paths, silently excluding most performance, regression, and smoke test files from tsc --noEmit. Either restore ./**/*.ts or add explicit patterns that cover the missing directories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/e2e/tsconfig.json, line 9:
<comment>The new include list drops ~75+ existing test files from type-checking. The old `./**/*.ts` wildcard ensured all e2e `.ts` files were compiled; the new explicit list only covers about 17 file paths, silently excluding most performance, regression, and smoke test files from `tsc --noEmit`. Either restore `./**/*.ts` or add explicit patterns that cover the missing directories.</comment>
<file context>
@@ -5,5 +5,16 @@
},
- "include": ["./**/*.ts"]
+ "include": [
+ "./performance/timeline-stability/**/*.spec.ts",
+ "./performance/timeline-stability/fixture.test.ts",
+ "./performance/timeline-stability/fixture.ts",
</file context>
| }, | ||
| }, | ||
| { deleteBeforeReplace: $app.stage !== "production" }, | ||
| { deleteBeforeReplace: $app.stage !== "production", ignoreChanges: ["metadata"] }, |
There was a problem hiding this comment.
P1: Adding ignoreChanges: ["metadata"] alongside new schema fields (model, model_tier, user_id) means Pulumi will skip applying metadata/schema changes on subsequent updates for already-deployed tables — so these new columns probably won't actually be created where the table already exists. If the intent is only to avoid the risky delete-before-replace behavior in production, consider a narrower approach (e.g., scoping ignoreChanges only when the table already has the desired schema, or handling schema evolution separately) so new columns still get applied.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At infra/stats.ts, line 89:
<comment>Adding `ignoreChanges: ["metadata"]` alongside new schema fields (model, model_tier, user_id) means Pulumi will skip applying metadata/schema changes on subsequent updates for already-deployed tables — so these new columns probably won't actually be created where the table already exists. If the intent is only to avoid the risky delete-before-replace behavior in production, consider a narrower approach (e.g., scoping ignoreChanges only when the table already has the desired schema, or handling schema evolution separately) so new columns still get applied.</comment>
<file context>
@@ -84,7 +86,7 @@ const inferenceEventTable = new aws.s3tables.Table(
},
},
- { deleteBeforeReplace: $app.stage !== "production" },
+ { deleteBeforeReplace: $app.stage !== "production", ignoreChanges: ["metadata"] },
)
</file context>
| recording.frames.push({ at: Date.now() - recording.startedAtEpoch, data: frame.data }) | ||
| await new Promise((resolve) => setTimeout(resolve, 50)) | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
P2: Capture-loop errors are silently swallowed — a CDP disconnect, screenshot failure, or any thrown error in the while-loop body goes completely invisible, making debugging of flaky or broken captures needlessly hard. Log the error (e.g. console.warn) in the catch block so capture failures are observable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/e2e/utils/visual-stability/capture.ts, line 35:
<comment>Capture-loop errors are silently swallowed — a CDP disconnect, screenshot failure, or any thrown error in the while-loop body goes completely invisible, making debugging of flaky or broken captures needlessly hard. Log the error (e.g. console.warn) in the catch block so capture failures are observable.</comment>
<file context>
@@ -0,0 +1,51 @@
+ recording.frames.push({ at: Date.now() - recording.startedAtEpoch, data: frame.data })
+ await new Promise((resolve) => setTimeout(resolve, 50))
+ }
+ } catch {
+ recording.running = false
+ }
</file context>
| return { | ||
| firstDestinationObservedMs: samples[firstDestination]?.observedAtMs ?? null, | ||
| firstCorrectObservedMs: samples[firstCorrect]?.observedAtMs ?? null, | ||
| stableObservedMs: samples[stable + 2]?.observedAtMs ?? null, |
There was a problem hiding this comment.
P2: When no stable three-sample window exists, stable is -1, so stable + 2 points at the second sample and falsely reports its timestamp as stableObservedMs. Guard the index before reading the sample so unsuccessful classification remains null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/e2e/performance/timeline/session-tab-switch-metrics.ts, line 25:
<comment>When no stable three-sample window exists, `stable` is `-1`, so `stable + 2` points at the second sample and falsely reports its timestamp as `stableObservedMs`. Guard the index before reading the sample so unsuccessful classification remains `null`.</comment>
<file context>
@@ -0,0 +1,59 @@
+ return {
+ firstDestinationObservedMs: samples[firstDestination]?.observedAtMs ?? null,
+ firstCorrectObservedMs: samples[firstCorrect]?.observedAtMs ?? null,
+ stableObservedMs: samples[stable + 2]?.observedAtMs ?? null,
+ wrongDestinationSamples: samples
+ .slice(firstDestination)
</file context>
Suggested change
| stableObservedMs: samples[stable + 2]?.observedAtMs ?? null, | |
| stableObservedMs: stable >= 0 ? (samples[stable + 2]?.observedAtMs ?? null) : null, |
| } | ||
| case "toString": { | ||
| const radix = optNum(0) | ||
| if (radix !== undefined && (radix < 2 || radix > 36)) { |
There was a problem hiding this comment.
P2: The manually-thrown radix-range error for Number.toString doesn't call .as("RangeError"), unlike the equivalent range-check pattern used elsewhere in stdlib (e.g. string.ts's normalize, url.ts, json.ts). Without it, errorName defaults to "Error", so catch (e) { e.name } in a CodeMode program will see "Error" instead of "RangeError", diverging from real JS semantics and from the sibling stdlib convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/codemode/src/stdlib/number.ts, line 29:
<comment>The manually-thrown radix-range error for `Number.toString` doesn't call `.as("RangeError")`, unlike the equivalent range-check pattern used elsewhere in stdlib (e.g. `string.ts`'s `normalize`, `url.ts`, `json.ts`). Without it, `errorName` defaults to `"Error"`, so `catch (e) { e.name }` in a CodeMode program will see `"Error"` instead of `"RangeError"`, diverging from real JS semantics and from the sibling stdlib convention.</comment>
<file context>
@@ -0,0 +1,66 @@
+ }
+ case "toString": {
+ const radix = optNum(0)
+ if (radix !== undefined && (radix < 2 || radix > 36)) {
+ throw new InterpreterRuntimeError("Number.toString radix must be between 2 and 36.", node)
+ }
</file context>
Suggested change
| if (radix !== undefined && (radix < 2 || radix > 36)) { | |
| if (radix !== undefined && (radix < 2 || radix > 36)) { | |
| throw new InterpreterRuntimeError("Number.toString radix must be between 2 and 36.", node).as("RangeError") | |
| } |
| @@ -127,6 +128,14 @@ function createServerCtx( | |||
| } | |||
|
|
|||
| const projectsList = createMemo(() => projects.list().map(enrich)) | |||
| const recentlyClosedList = createMemo(() => { | |||
There was a problem hiding this comment.
P3: This recentlyClosed computation (filter known worktrees via pathKey, slice to RECENTLY_CLOSED_DISPLAY_LIMIT, map through enrich) duplicates the identical logic already in layout.tsx's projects.recentlyClosed. Consider extracting a shared helper (e.g. in server.ts) so both contexts stay in sync if the filtering/limit logic changes later.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/src/context/global.tsx, line 131:
<comment>This recentlyClosed computation (filter known worktrees via pathKey, slice to RECENTLY_CLOSED_DISPLAY_LIMIT, map through enrich) duplicates the identical logic already in layout.tsx's `projects.recentlyClosed`. Consider extracting a shared helper (e.g. in server.ts) so both contexts stay in sync if the filtering/limit logic changes later.</comment>
<file context>
@@ -127,6 +128,14 @@ function createServerCtx(
}
const projectsList = createMemo(() => projects.list().map(enrich))
+ const recentlyClosedList = createMemo(() => {
+ const known = new Set(sync.data.project.map((project) => pathKey(project.worktree)))
+ return projects
</file context>
| keybindParts: (id: string) => string[] | ||
| } | ||
|
|
||
| export function reviewTooltipKeybind(command: CommandKeybind, _translate?: (key: string) => string) { |
There was a problem hiding this comment.
P3: Both reviewTooltipKeybind and newTabTooltipKeybind accept a _translate callback that is never called, so any intended localization of keybind labels is silently dropped. Either wire the translate function into the returned keybind parts or drop the unused parameter to avoid misleading callers/tests into thinking translation is applied.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/src/components/command-tooltip-keybind.ts, line 5:
<comment>Both `reviewTooltipKeybind` and `newTabTooltipKeybind` accept a `_translate` callback that is never called, so any intended localization of keybind labels is silently dropped. Either wire the translate function into the returned keybind parts or drop the unused parameter to avoid misleading callers/tests into thinking translation is applied.</comment>
<file context>
@@ -0,0 +1,11 @@
+ keybindParts: (id: string) => string[]
+}
+
+export function reviewTooltipKeybind(command: CommandKeybind, _translate?: (key: string) => string) {
+ return command.keybindParts("review.toggle")
+}
</file context>
| @@ -51,6 +62,13 @@ export const ModelTooltip: Component<{ model: ModelInfo; latest?: boolean; free? | |||
| const suffix = tags.length ? ` (${tags.join(", ")})` : "" | |||
| return `${sourceName(props.model)} ${props.model.name}${suffix}` | |||
| } | |||
| const name = () => { | |||
There was a problem hiding this comment.
P3: name() duplicates the tag-building logic already in title() just above it; consider extracting a shared tagSuffix() helper used by both to avoid the two implementations drifting apart later.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/src/components/model-tooltip.tsx, line 65:
<comment>`name()` duplicates the tag-building logic already in `title()` just above it; consider extracting a shared `tagSuffix()` helper used by both to avoid the two implementations drifting apart later.</comment>
<file context>
@@ -51,6 +62,13 @@ export const ModelTooltip: Component<{ model: ModelInfo; latest?: boolean; free?
const suffix = tags.length ? ` (${tags.join(", ")})` : ""
return `${sourceName(props.model)} ${props.model.name}${suffix}`
}
+ const name = () => {
+ const tags: Array<string> = []
+ if (props.latest) tags.push(language.t("model.tag.latest"))
</file context>
| applyingHistory: boolean | ||
| } | ||
|
|
||
| function resetPromptInputTransientState(setStore: SetStoreFunction<PromptInputTransientState>) { |
There was a problem hiding this comment.
P3: The reset defaults and the initial store defaults are duplicated in two places; consider extracting a shared DEFAULTS object (excluding placeholder) and spreading it in both createStore and resetPromptInputTransientState to avoid future drift when new fields are added.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/src/components/prompt-input/transient-state.ts, line 17:
<comment>The reset defaults and the initial store defaults are duplicated in two places; consider extracting a shared `DEFAULTS` object (excluding `placeholder`) and spreading it in both `createStore` and `resetPromptInputTransientState` to avoid future drift when new fields are added.</comment>
<file context>
@@ -0,0 +1,46 @@
+ applyingHistory: boolean
+}
+
+function resetPromptInputTransientState(setStore: SetStoreFunction<PromptInputTransientState>) {
+ setStore({
+ popover: null,
</file context>
| <main class="flex-1 min-h-0 min-w-0 overflow-x-hidden flex flex-col items-start contain-strict"> | ||
| <Suspense>{props.children}</Suspense> | ||
| </main> | ||
| {import.meta.env.DEV && state.debugTools && <DebugBar inline />} |
There was a problem hiding this comment.
P3: The new layout drops the VITE_DISABLE_DEBUG_BAR env check that the legacy layout uses before rendering DebugBar, so setting that env var will no longer hide the debug bar when the new layout is active, creating inconsistent behavior between the two layouts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app/src/pages/layout-new.tsx, line 48:
<comment>The new layout drops the `VITE_DISABLE_DEBUG_BAR` env check that the legacy layout uses before rendering `DebugBar`, so setting that env var will no longer hide the debug bar when the new layout is active, creating inconsistent behavior between the two layouts.</comment>
<file context>
@@ -0,0 +1,53 @@
+ <main class="flex-1 min-h-0 min-w-0 overflow-x-hidden flex flex-col items-start contain-strict">
+ <Suspense>{props.children}</Suspense>
+ </main>
+ {import.meta.env.DEV && state.debugTools && <DebugBar inline />}
+ <TabsInfoPopup />
+ <ToastRegion v2 />
</file context>
Suggested change
| {import.meta.env.DEV && state.debugTools && <DebugBar inline />} | |
| {import.meta.env.DEV && import.meta.env.VITE_DISABLE_DEBUG_BAR !== "1" && state.debugTools && <DebugBar inline />} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
19 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings anomalyco/opencode up to v1.18.4 (
d36a2d898on dev). 921 upstream commits since8716c4309(v1.17.8, 2026-06-19).Major upstream themes: layer-node wiring completion (all runtimes/tests built from
LayerNodegraphs,defaultLayerexports removed), plugin boot moved fromcore/src/plugin/boot.tsintoplugin/internal.ts, OAuth callback HTML centralized intocore/src/oauth/page.ts, tree-sitter worker embedded as text inbuild.ts, gatedreferenceDirs, experimentalcodemodetool.Conflicts resolved (28)
.github/workflows/*.yml,README.{zh,zht}.md,customize-opencode.md,core/test/plugin/skill.test.tsbun.lockbun installpackage.json(root)packages/opencode/package.json@browser-use/browsercode-core, took version 1.18.4AGENTS.mdcore/src/plugin/boot.tsskill.ts)core/src/plugin/skill.tsdefineshape (customize-opencode skill stays removed)app/index.html,ui/site.webmanifestapp/titlebar.tsxtui/tips-view.tsxopencode/script/build.tsbcode-asset naming, took treeSitterWorker text embedopencode/src/agent/agent.tsreferenceDirsopencode/src/installation/index.tsopencode/src/tool/registry.tsdefaultLayerwith upstream;nodenow pipesFetchUse.layerinto its own layer (httpClient dep satisfies it)test/session/prompt.test.tstest/tool/webfetch.test.tsConfig.nodeadded to group + FetchUse layer merged (our webfetch routing needs both)mcp/oauth-callback.ts,plugin/openai/codex.tsBranding note
Upstream centralized all OAuth callback pages into
core/src/oauth/page.ts, which carries OpenCode branding (wordmark SVG + copy). Our previous per-file BrowserCode HTML blobs are gone with this sync. These pages are browser-facing (OAuth flows only), and fork policy is to rebrand terminal-facing surfaces only — so this ships as upstream. If we ever want it back, it is now a single-file change instead of three.Step 3a
Six reintroduced upstream workflows re-deleted during conflict resolution;
.github/workflows/remainsrelease.yml+typecheck.ymlonly. No new unclassified workflows this window.Verification
bun install: cleanbun run typecheck(filtered): 16/16 passedtest/tool/webfetch.test.ts: 4/4 passtest/session/prompt.test.ts: ~30 failures on the dev Mac — pre-existing, not a sync regression: our pre-syncmainfails identically (29) while pristine upstreamd36a2d898passes 56/56 on the same machine. A fork divergence hangs these tests locally; follow-up issue recommended.Yellow-zone audit
Upstream touched 15 Yellow files this window (
AGENTS.md,core/src/global.ts,script/build.ts,agent.ts,config.ts,installation/index.ts,mcp/index.ts,plugin/index.ts,provider.ts,session/{processor,session}.ts,skill/index.ts,tool/registry.ts,tui/app.tsx,tui/routes/session/index.tsx). All BrowserCode customizations verified present post-merge:app = "bcode", bcode config filenames +bcode.shschema/install URLs, provider attribution headers, Laminar plugin, FetchUse wiring, skills materialization, browser_execute registry + TUI display, typed omitted-image messages.Summary by cubic
Sync to upstream v1.18.4. Adopts the new LayerNode runtime wiring, centralized OAuth page, plugin boot relocation, and tree‑sitter worker embedding, while keeping BrowserCode customizations and pruning upstream workflows.
New Features
LayerNodegraphs;defaultLayerexports removed.core/src/oauth/page.ts(ships upstream branding per policy).build.tsembeds the tree‑sitter worker; gatedreferenceDirs; experimentalcodemode.packages/app/e2e/performance(Playwright configs, timeline/navigation probes, Chrome trace capture).AGENTS.mdand client codegen notes.Dependencies
@effect/opentelemetry,@effect/platform-node,@effect/sql-sqlite-bunto4.0.0-beta.83; refreshed rootbun.lock.@remotion/cli,react@19,remotion@4forartifacts/glm52-rise-video..github/actions/setup-bunensures Node 24 fornode-gyp; addedSUPPORT_API_KEYsecret and links; Athena query scan cap; Linux desktop entry; relaxed Bun check in Nix.packages/client/src/generated/andpackages/client/src/generated-effect/to.prettierignore.Written for commit c761eee. Summary will update on new commits.