Skip to content

Desktop /review reuses a cached merge base after HEAD changes #35667

Description

@mhemmings

Summary

In Codex Desktop, starting a new base-branch review after changing HEAD can produce a new review prompt containing the merge-base SHA from the previous checkout.

This supersedes #30751. That report describes the correct symptom, but attributes it to prompt reuse/timing in the public Rust review code. Inspection of the installed desktop bundle shows that the stale SHA originates in the desktop client's Git query cache before the prompt is constructed.

Environment

  • Codex Desktop: 26.721.41059 (5848)
  • Bundle ID: com.openai.codex
  • macOS 14.6.1 (23G93), arm64

Reproduction

  1. In one repository, check out branch A.
  2. Start /review against origin/main, causing merge base A to be resolved.
  3. Change HEAD to branch B outside the app. Branch B must have a different merge base with origin/main.
  4. Without restarting Codex, start another /review against origin/main.
  5. Inspect the generated review instructions.

The second, newly generated prompt can still contain merge base A. Running git merge-base HEAD origin/main directly returns merge base B.

Root cause in the desktop bundle

The renderer asks the Electron main process for a merge base and interpolates the returned SHA into a plain-text prompt:

let result = await invoke("git-merge-base", {
  source: "review_model",
  params: { gitRoot, baseBranch, hostId },
});

prompt = reviewPrompt
  .replaceAll("{baseBranch}", baseBranch)
  .replaceAll("{mergeBaseSha}", result.mergeBaseSha.trim());

It then submits that text through the ordinary start-turn-for-host or start-conversation path. It does not call the app server's review/start endpoint.

The underlying query is effectively:

fetchQuery({
  queryKey: ["git", hostId, repoRoot, "head-merge-base", baseBranch],
  queryFn: () => git("merge-base", "HEAD", baseBranch),
  staleTime: Infinity,
});

The key contains the repository and base branch, but neither the resolved HEAD OID nor the resolved base-ref OID. Consequently, branch A and branch B use the same key. The Git watcher runs in a separate worker with a separate Git manager/query client, so its head/ref invalidation does not invalidate the main-process cache used by the renderer's git-merge-base IPC handler.

The result is not reuse of an old prompt: the desktop constructs a new prompt using an old cached query result.

Expected behavior

Every new base-branch review should be scoped using the repository state current when the review starts. A previously cached merge base must not survive a HEAD or base-ref change.

Recommended fix

Remove desktop-side merge-base and prompt resolution from /review. Send the semantic target through the app server's existing API:

{
  "method": "review/start",
  "params": {
    "threadId": "",
    "target": {
      "type": "baseBranch",
      "branch": "origin/main"
    },
    "delivery": "inline"
  }
}

The public app-server protocol already accepts ReviewTarget::BaseBranch, and the server resolves it against the current repository state. This also removes the duplicated review prompt and the cross-process cache correctness dependency from the desktop client.

A smaller fix would be to invalidate the same main-process query client on HEAD/ref changes, or include the resolved HEAD and base-ref OIDs in the key. Calling review/start is preferable because review-target resolution already belongs to that API.

Regression test

In one desktop/app process:

  1. Start a review of branch A against a base branch and record merge base A.
  2. Externally check out branch B, whose merge base is B.
  3. Start another review against the same base branch.
  4. Assert that the second review either dispatches a semantic review/start target or contains merge base B, and never merge base A.

Relationship to #30751

The candidate change linked from #30751 modifies codex-rs/prompts/src/review_request.rs so the model computes the merge base during the review turn. That can address a separate prompt-to-execution timing race in clients using the public review path. It does not fix this desktop bug because the desktop slash-command path constructs and dispatches its own prompt without invoking that resolver.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    appIssues related to the Codex desktop appbugSomething isn't workingcode-reviewIssues relating to code reviews performed by codex

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions