Fix resource refresh button not triggering re-fetch

[olaservo]

Summary

Fixes #1120

• The readResource function had an early return when the resource URI was already in resourceContentMap, which prevented the Refresh button from ever re-fetching
• Added a { bypassCache } options parameter to readResource to skip the cache when the Refresh button is clicked
• When not bypassing cache, cached content is now properly synced to the display (fixes switching between previously viewed resources showing stale content)
• Errors from failed resource reads are no longer cached, so re-clicking a resource will retry instead of serving a stale error

Changes

• client/src/App.tsx — Added { bypassCache } options parameter to readResource; stopped caching errors in resourceContentMap; forwarded options through the prop wrapper
• client/src/components/ResourcesTab.tsx — Updated type signature; Refresh button now passes { bypassCache: true }

Test plan

• Existing unit tests pass
• TypeScript type-check passes (npx tsc --noEmit)
• Build passes (npm run build-client)
• Lint passes (npm run lint)
• Manual verification: connect to an MCP server, click a resource, click Refresh → new request appears in history
• Manual verification: click resource A, click resource B, click resource A again → correct cached content displays
• Manual verification: simulate a failed resource read, then re-click → resource is retried instead of showing stale error

🦉 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes resource re-fetch behavior in the client by adding a bypassCache option to readResource, ensuring the Refresh button triggers a new server request and cached content is correctly synced to the UI when revisiting previously viewed resources.

Changes:

• Add { bypassCache } option to readResource and use it to bypass cache on Refresh.
• Sync cached resource content to the display instead of early-returning without updating UI state.
• Stop caching read errors in resourceContentMap (intended to ensure retries on re-click).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
client/src/App.tsx	Adds bypassCache support to readResource, updates caching/refresh logic, and forwards options through the ResourcesTab prop wrapper.
client/src/components/ResourcesTab.tsx	Updates readResource prop signature and passes { bypassCache: true } from the Refresh button.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Because resourceErrorMap[uri] is only cleared on success, any retry attempt will continue to have an error entry until the request completes. Since some UI paths render resourceError in preference to cached content, this can leave a stale error visible while the retry is in-flight. Consider clearing resourceErrorMap[uri] when starting a new read (after passing the cache check / before sending the request), not only when a read succeeds.

Suggested change

	setResourceErrorMap((prev) => {
	if (!hasOwn.call(prev, uri)) return prev;
	const next = { ...prev };
	delete next[uri];
	return next;
	});

[olaservo]

Addressed in 01ea4cb — moved resourceErrorMap[uri] clearing from the success handler to right after the cache-hit check (before sending the request). During an in-flight retry, the UI now shows last-known-good content instead of the stale error.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

errorString uses nullish coalescing (??), so an Error with an empty message (e.g. new Error("")) will produce an empty string and render a blank error. Prefer a fallback that treats empty messages as missing (e.g. use || and/or include error.name).

[olaservo]

Addressed in f879c0d — switched from ?? to || so an empty .message falls through to String(error), which always produces a non-empty representation.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

fetchingResources is React state, so fetchingResources.has(uri) can be stale if readResource is triggered twice before the state update is applied (e.g., double-click Refresh / rapid re-select). That can lead to duplicate resources/read requests for the same URI. Consider tracking in-flight URIs in a useRef (or similar) so the guard is synchronous and cannot race with state updates.

[olaservo]

Valid observation, but this is a pre-existing issue — fetchingResources was already React state before this PR. The race condition on rapid clicks applies to the original code as well. A useRef-based guard would be a good improvement, but it's out of scope for this bug fix and should be addressed in a separate PR.

[Copilot]

The new cache-bypass and error-not-cached behavior in readResource is central to fixing #1120, but it’s currently only covered indirectly via ResourcesTab prop tests (it doesn’t assert that App actually re-fetches when cached, nor that errors aren’t cached). Consider adding an App-level unit/integration test that verifies: (1) cache hits sync resourceContent without calling makeRequest, (2) { bypassCache: true } forces a re-fetch even when cached, and (3) failed reads don’t populate resourceContentMap so a subsequent click retries.

[olaservo]

Valid suggestion. The ResourcesTab prop tests verify that the correct arguments (bypassCache: true for Refresh, no options for normal clicks) are passed, but App-level integration tests verifying the actual cache/retry behavior would add more confidence. This would require mocking sendMCPRequest and the full App rendering context — better suited as a follow-up rather than this bug fix PR.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

readResource now calls setResourceContent(resourceContentMap[uri]) on a cache hit. When readResource is triggered from non-Resources contexts (e.g. Tools tab expanding a resource_link), this will update the global resourceContent string without updating selectedResource, so the Resources tab can later display content that doesn’t match the selected resource header. Consider only syncing resourceContent on cache hits when the current tab is resources (or when selectedResource?.uri === uri), and otherwise just return without touching resourceContent.

Suggested change

	setResourceContent(resourceContentMap[uri]);
	if (currentTabRef.current === "resources") {
	setResourceContent(resourceContentMap[uri]);
	}

[olaservo]

Addressed in fca508c — guarded the cache-hit setResourceContent call with currentTabRef.current === "resources" so expanding a resource_link in the Tools tab no longer overwrites the Resources tab's displayed content.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.