Desktop `/review` reuses a cached merge base after HEAD changes

Open
#35,667 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
38/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
electron, rust
Domain
api, desktop

Research direction

Trace the desktop /review renderer and Electron main-process git-merge-base IPC handler, focusing on the head-merge-base query key and its separate query client. Reproduce the branch A/branch B sequence first, then inspect the existing review/start path and the desktop regression-test location. Done means a new review uses the current merge base or dispatches a semantic target, with a test proving the old SHA is not reused.

Written by the indexing model from the issue text.

Description

app bug code-review

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.

Dominant language
Rust
Stars
125k
Forks
19.5k
Avg merge
1m
Merged PRs (30d)
1k

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from openai/codex

All issues in openai/codex

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.