Skip to content

Fix cache corruption from partial updates in file refresh #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
shuv1337 merged 2 commits into from
Nov 14, 2025
Merged

Fix cache corruption from partial updates in file refresh #18

shuv1337 merged 2 commits into from
Nov 14, 2025

Conversation

Copy link

Copilot AI commented Nov 14, 2025 •
edited by cursor bot
Loading

The refresh function passed the shared cache object to fetchEntries, which mutated it in-place. If file system operations failed mid-iteration, the cache was left partially updated but marked as complete.

Changes:

  • Build cache entries in a temporary object, only assigning to shared cache on success
  • Remove parameter passing since fetchEntries no longer mutates external state
// Before: mutates shared cache directly
const fetchEntries = async (result: Entry) => {
  for await (const file of Ripgrep.files({ cwd: Instance.directory })) {
    result.files.push(file)  // partial update on error
  }
  cache = result
}

// After: atomic update on success
const fetchEntries = async () => {
  const temp: Entry = { files: [], dirs: [] }
  for await (const file of Ripgrep.files({ cwd: Instance.directory })) {
    temp.files.push(file)
  }
  cache = temp  // only reached if loop completes
}

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Note

Make file list refresh atomic by building a temp cache and updating cache only on success; adjust refresh API and call sites.

  • File cache refresh (packages/opencode/src/file/index.ts):
    • Build entries in a temporary Entry within fetchEntries() and assign to cache only after completion.
    • Remove result parameter from fetchEntries; update refresh() and its call sites (state init and files() method) accordingly.
    • Maintain fetching guard and perform background refresh without mutating shared state during iteration.

Written by Cursor Bugbot for commit 416719a. This will update automatically on new commits. Configure here.

Copilot AI mentioned this pull request Nov 14, 2025
Merged
@shuv1337
Fix cache corruption bug in refresh function
416719a 
Co-authored-by: kcrommett <523952+kcrommett@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix cache corruption issue from incomplete hot reload Fix cache corruption from partial updates in file refresh Nov 14, 2025
Copilot AI requested a review from shuv1337 November 14, 2025 09:34
@shuv1337 shuv1337 marked this pull request as ready for review November 14, 2025 09:43
@shuv1337 shuv1337 merged commit 37527bd into config-hot-reload Nov 14, 2025
1 check passed
shuv1337 added a commit that referenced this pull request Nov 17, 2025
@shuv1337 @actions-user
Config hot reload (#20)
44217bb 
* PATCH /config hot-reload w/ selective cache invalidation (#15)

PATCH /config hot-reload w/ selective cache invalidation

* typecheck fix

* chore: format code

* fix for failing test

* fix for failing test

* chore: format code

* chore: format code

* Fix cache corruption from partial updates in file refresh (#18)

* Fix cache corruption bug in refresh function

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
@shuv1337 shuv1337 deleted the copilot/sub-pr-17 branch November 21, 2025 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shuv1337 shuv1337 Awaiting requested review from shuv1337

Assignees

@shuv1337 shuv1337

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shuv1337