fix(client): project icons update after a turn rewrites them - #14829
StanMan007 wants to merge 1 commit into
Conversation
A project's icon URL was only re-requested every 30 minutes, so an icon file an agent rewrote kept showing the old image until then or a reload. Ask for it again whenever one of the project's turns finishes; the server URL is content-hashed, so an unchanged icon keeps its cached image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| const latest = latestProjectTurnCompletedAt(get(shellState(environmentId)), cwd); | ||
| const previous = Option.getOrNull(get.self<string | null>()); | ||
| return previous !== null && (latest === null || previous > latest) | ||
| ? previous |
There was a problem hiding this comment.
🟡 Medium state/assets.ts:252
Two turns that complete in separate shell updates with the same completedAt produce the same refresh signal, so Atom.makeRefreshOnSignal does not re-request the favicon after the second turn. Include a per-turn identity such as turnId, or a monotonically increasing revision, in the signal so every completion invalidates the cached URL.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/assets.ts around line 252:
Two turns that complete in separate shell updates with the same `completedAt` produce the same refresh signal, so `Atom.makeRefreshOnSignal` does not re-request the favicon after the second turn. Include a per-turn identity such as `turnId`, or a monotonically increasing revision, in the signal so every completion invalidates the cached URL.
There was a problem hiding this comment.
Not changing this. It needs two turns in the same project to finish at the identical millisecond and arrive in separate shell updates. Even then, the first completion has already re-requested the icon, so at most a rewrite made by the second of two simultaneous turns waits for the next completion or the regular 30-minute refresh. Adding turnId would also break the forward-only ordering that keeps a turn starting from triggering a refetch, because IDs don't sort by time.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a contained client-side bug fix that refreshes existing project-icon requests after relevant turns, with focused test coverage and no schema, deployment, security, billing, or default-setting changes. An unresolved Medium finding does identify an edge case where equal completion timestamps can suppress a later refresh and should be accounted for separately. Notes:
You can add or adjust custom eligibility rules. Learn more. |
|
Note This comment is posted by Julius' dot Closing for missing UI verification. This changes when the sidebar and project switcher show a rewritten icon, but the PR explicitly says no before/after screenshots or recording were captured. The focused atom test checks the URL refresh; it doesn't show the visible update after a turn completes. The verification policy requires before/after images and a short recording for this timing behavior. Attach that evidence, name the client you checked, and request reconsideration. |
|
Added the missing UI verification. The PR description now has a UI check (web client) section with before/after screenshots and recordings.
Requesting reconsideration. |
Problem
When a project's icon file is rewritten on disk, for example after an agent replaces the contents of
favicon.svg, the sidebar and project switcher keep showing the old icon for up to 30 minutes, or until the app reloads. The client re-requests a project's icon URL only on its 30-minute refresh interval. The server already content-hashes the URL, so asking again is enough to pick up the change.Repro: open a project with a
favicon.svg, ask an agent to rewrite it, and wait for the turn to finish. The old icon stays until a reload.Change
createProjectFaviconUrlAtomFamilytakes an optionalshellStateinput. It re-requests a project's icon whenever one of that project's turns completes, using the sameAtom.makeRefreshOnSignalpattern the clone-phase signal already uses.apps/webandapps/mobile, which share the client-runtime family.Scope and approval
There is no prior issue. This is a small fix for an obvious staleness bug. It adds a single refresh trigger to the existing favicon query, mirrors the clone-refresh trigger from #14635, and does not change the server or the cache format.
Verification
asks for a project's icon again when one of its turns finishesinpackages/client-runtime/src/state/assets.test.ts. It checks that:vp test run src/state/assets.test.ts(client-runtime): 22 passed.ProjectFavicon,ProjectFaviconPickerDialogandattachmentUploadQueuetests: 31 passed.state/assets: 46 passed.vp fmt,vp lintandtsc --noEmitforclient-runtime,webandmobile: clean.Known limit, not changed here: the server caches icon discovery, which a pre-push review flagged. Hits are cached for 10 minutes and misses for 1 minute, so a brand-new icon file, a new higher-priority candidate, or a changed
t3.jsoniconPathis still picked up only when that cache expires. Rewriting an already-discovered file works, because the content hash is computed on every request. Fixing discovery would need a contract or server change, so I kept it out of this one-problem PR.UI check (web client)
I ran the web client from this branch's local dev build, using its own throwaway
.t3state, in Chrome via the T3 Code browser panel. The demo project has a redfavicon.svg. In a new thread, the same prompt asks the agent to overwrite it with a blue square. The project icon appears in the thread header and the sidebar rows.upstream/mainclient code forpackages/client-runtime/src/state/assets.tsandapps/web/src/state/assets.ts): the turn finishes and the file on disk is blue, but both icons stay red.Full-speed recordings: before.mp4 and after.mp4. They are hosted as release assets on my fork because the PR comment uploader wasn't available from the automation browser. Mobile was not checked visually; it shares the same client-runtime family, and its unit suites above pass.
Made with Claude Opus 5.5 (
claude-opus-5-5) in Claude Code, running inside T3 Code.🤖 Generated with Claude Code