Skip to content

chore(sync): preview upstream/dev (20260206)#10

Closed
aryasaatvik wants to merge 13 commits intodevfrom
codex/merge-upstream-dev-20260206
Closed

chore(sync): preview upstream/dev (20260206)#10
aryasaatvik wants to merge 13 commits intodevfrom
codex/merge-upstream-dev-20260206

Conversation

@aryasaatvik
Copy link
Member

@aryasaatvik aryasaatvik commented Feb 6, 2026

Review-friendly preview of syncing upstream/dev into dev.

Note: GitHub Actions is non-blocking for this workflow. Local checks are the source of truth.

Apply branch (linear rebase): codex/rebase-upstream-dev-20260206
Upstream: 0ec5f66
Origin dev (before): 2b326cd

Apply:

cd /Users/aryasaatvik/Developer/opencode-worktrees/rebase
git push --force-with-lease origin codex/rebase-upstream-dev-20260206:dev

Open with Devin

aryasaatvik and others added 13 commits February 6, 2026 11:55
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
# Conflicts:
#	packages/app/src/components/dialog-select-file.tsx
#	packages/app/src/pages/layout.tsx
#	packages/app/src/pages/session.tsx
#	packages/desktop/src/index.tsx
@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

View 8 additional findings in Devin Review.

Open in Devin Review

Comment on lines 187 to 205
const sessionToken = { value: 0 }
let sessionInflight: Promise<Entry[]> | undefined
let sessionAll: Entry[] | undefined

const sessions = (text: string) => {
const query = text.trim()
if (!query) {
sessionToken.value += 1
sessionInflight = undefined
sessionAll = undefined
return [] as Entry[]
}

if (sessionAll) return sessionAll
if (sessionInflight) return sessionInflight

const current = sessionToken.value
const dirs = workspaces()
if (dirs.length === 0) return [] as Entry[]

Choose a reason for hiding this comment

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

🟡 Session search results can go stale because session cache isn’t keyed by workspace/project

The session candidate list in the file picker dialog is cached globally (sessionAll) after the first non-empty query, but the cache is not invalidated when the underlying workspace set changes.

Root Cause

sessions(text) only resets its cache when the query becomes empty (it clears sessionAll/sessionInflight in the if (!query) branch), and otherwise returns sessionAll for all subsequent queries:

  • Cache is reused: if (sessionAll) return sessionAll (packages/app/src/components/dialog-select-file.tsx:200-205).
  • sessionAll is populated from workspaces() at fetch time (packages/app/src/components/dialog-select-file.tsx:203-239).

If the dialog stays open while navigation changes params.dir (or layout.projects.list() changes) such that workspaces() changes, the dialog can keep showing sessions from the previous workspace set.

Actual: cached sessions from prior workspace(s) may appear.

Expected: cache should be invalidated when workspaces() (or at least params.dir) changes.

Impact: confusing/incorrect session navigation suggestions; user may navigate into unexpected projects/sessions.

Prompt for agents
In packages/app/src/components/dialog-select-file.tsx, invalidate the session cache when the workspace set changes. Concretely: add a createEffect/on that watches params.dir (and/or projectDirectory(), workspaces()) and, when it changes, increments sessionToken.value and clears sessionInflight/sessionAll (same logic as the empty-query branch). This ensures sessions() refetches for the current workspace context even if the search query stays non-empty.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 376 to 391
window.__OPENCODE__ ??= {}
window.__OPENCODE__.serverPassword = data().password ?? undefined

return <AppInterface defaultUrl={data().url} />
function Inner() {
const cmd = useCommand()

menuTrigger = (id) => cmd.trigger(id)

return null
}

return (
<AppInterface defaultUrl={data().url}>
<Inner />
</AppInterface>
)

Choose a reason for hiding this comment

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

🟡 Desktop menu actions can be no-ops (and handler can go stale) because menuTrigger is set without lifecycle management

The desktop app wires native menu items to Solid commands via a module-level menuTrigger function. That function is assigned from inside an Inner component and is never cleared, and it can be null during early startup.

Root Cause
  • createMenu is created immediately and calls menuTrigger?.(id) (packages/desktop/src/index.tsx:345-348).
  • menuTrigger is assigned later in Inner() via menuTrigger = (id) => cmd.trigger(id) (packages/desktop/src/index.tsx:379-385).

If a user triggers a menu item before Inner has run, the action is dropped (no retry). Additionally, because there’s no onCleanup resetting menuTrigger, it can point at a command context that has been torn down (e.g., during HMR, or if the app remounts AppInterface).

Actual: menu items may intermittently do nothing, or may call into a stale command instance.

Expected: menu trigger should be registered/unregistered with component lifecycle, and menu actions should have a reliable handler once the app is ready.

Impact: unreliable desktop menu behavior.

(Refers to lines 345-391)

Prompt for agents
In packages/desktop/src/index.tsx, manage menuTrigger with lifecycle: inside Inner(), set menuTrigger in onMount (or immediately) and add onCleanup to reset it back to null if it still points to that instance. Optionally buffer menu events until menuTrigger is set (e.g., queue ids and flush once Inner mounts) so early menu interactions aren’t dropped.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@chatgpt-codex-connector
Copy link

💡 Codex Review

if (sessionAll) return sessionAll
if (sessionInflight) return sessionInflight

P2 Badge Avoid caching failed session list as permanent empty

Because sessions() short-circuits on any truthy sessionAll, a transient fetch failure (the per-directory .catch(() => []) below) can set sessionAll to [] and then permanently suppress retries while the query remains non-empty. An empty array is truthy, so subsequent searches in the same dialog session return [] even after connectivity recovers. Consider tracking errors separately or only caching sessionAll when at least one directory fetch succeeds, otherwise clear the cache on failure.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@aryasaatvik aryasaatvik closed this Feb 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant