Skip to content

fix: ignore disableCache in client#481

Merged
wan9chi merged 1 commit into
mainfrom
codex/disablecache-client-noop
Jun 21, 2026
Merged

fix: ignore disableCache in client#481
wan9chi merged 1 commit into
mainfrom
codex/disablecache-client-noop

Conversation

@wan9chi

@wan9chi wan9chi commented Jun 21, 2026

Copy link
Copy Markdown
Member

Summary

Temporarily make vite_task_client::Client::disable_cache() a no-op while we redesign the disableCache contract.

The server-side Request::DisableCache handling remains intact and covered by a raw protocol integration test. The public client method keeps its existing API shape so consumers do not need code changes.

Why

Vite/Vitest can currently call disableCache() too early, during server/config setup. That causes cacheable tasks such as vitest run to opt out even when they never listen on a port or observe the filesystem.

This PR avoids the false opt-out immediately. The real fix should move disableCache() calls to the exact operations that make a task uncacheable, such as port listening and filesystem observation.

Validation

  • mise exec -- cargo fmt --all --check
  • mise exec -- cargo check -p vite_task_client -p vite_task_server
  • mise exec -- cargo test -p vite_task_server --test integration
  • mise exec -- cargo clippy -p vite_task_client -p vite_task_server --all-targets --all-features -- --deny warnings

wan9chi commented Jun 21, 2026

Copy link
Copy Markdown
Member Author

Follow-up issue for the real disableCache contract and Vite/Vitest operation-boundary fix: #482

@wan9chi wan9chi force-pushed the codex/disablecache-client-noop branch from c1f3b00 to 770bc04 Compare June 21, 2026 13:07
@wan9chi wan9chi force-pushed the codex/disablecache-client-noop branch from 770bc04 to b0a7019 Compare June 21, 2026 13:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1f3b005d3

ℹ️ 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".

)]
pub fn disable_cache(&self) -> io::Result<()> {
self.send(&Request::DisableCache)
Ok(())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Update stale disableCache e2e snapshots

With this new Ok(()) path, public clients no longer send Request::DisableCache, but the include-ignored e2e fixture still asserts that disableCache() forces the next run to miss; running cargo test -p vite_task_bin --test e2e_snapshots -- --include-ignored disable_cache_forces_reexecution now fails because the second run is a cache hit. Please update or remove the affected ipc_client_test/vite_dev_disable_cache snapshots alongside this behavior change so the documented Node-client suite matches the new contract.

Useful? React with 👍 / 👎.

@wan9chi wan9chi merged commit 8daa9bb into main Jun 21, 2026
35 checks passed
@wan9chi wan9chi deleted the codex/disablecache-client-noop branch June 21, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant