feat(core): add cache-busting version param to apps-sdk widget output…#531
Open
slackerzz wants to merge 2 commits intoalpic-ai:mainfrom
Open
feat(core): add cache-busting version param to apps-sdk widget output…#531slackerzz wants to merge 2 commits intoalpic-ai:mainfrom
slackerzz wants to merge 2 commits intoalpic-ai:mainfrom
Conversation
d81ccb7 to
bb55dd9
Compare
Contributor
|
Thanks for the PR @slackerzz ! I'm going to do small tests to verify that it doesn't break any behavior. We have a small chat this morning with @fredericbarthelet - I'll update you ASAP ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this will close #530
as suggest by @qchuchu I added a cache-busting parameter to the outputTemplate uri
Greptile Summary
This PR adds a
?v=${Date.now()}cache-busting query parameter to theapps-sdkwidget URI inregisterWidget, so that each server startup produces a newopenai/outputTemplatevalue, preventing stale widget content from being served to clients that cache the URI.Changes:
server.ts: ThewidgetConfig.urifor theapps-sdkhost now appends?v=${Date.now()}at widget-registration time. Because the same URI value is assigned to both the MCP resource registration and theopenai/outputTemplatetool metadata, the two stay in sync.widget.test.ts: The existingoutputTemplateassertion is updated to expect the versioned URI. Avi.spyOn(Date, "now").mockReturnValue(now)is introduced to make the timestamp deterministic in the test.Issue found:
vi.spyOn(Date, "now")mock in the new test is not restored by the existingafterEach(() => vi.clearAllMocks()).clearAllMocksresets call history only — it does not restore the originalDate.nowimplementation. Any tests added after this one in the same describe block will receive the frozen timestamp instead of the real current time. TheafterEachshould be updated to callvi.restoreAllMocks()as well.Confidence Score: 4/5
server.tsis minimal and correct — a single-line addition of a timestamp query parameter that is consistently applied to both the resource URI and the tool metadata. The only real concern is in the test: thevi.spyOn(Date, "now")mock is not restored byvi.clearAllMocks(), which could silently break future tests. This is a logic-level test infrastructure issue, not a production bug, hence a score of 4 rather than 5.packages/core/src/test/widget.test.ts— theDate.nowspy introduced in the new test needs proper teardown viavi.restoreAllMocks().Last reviewed commit: d81ccb7
(2/5) Greptile learns from your feedback when you react with thumbs up/down!