Sign up
View stargazersSign up

fix(server): cache favicon lookups for missing project folders#23721

open2
main
GitHub fork: fix/23658-favicon-missing-root-cache
opened19d agobyFfixfon
3 files changed
View on GitHubTwo-way sync enabledPR last imported 15h ago

Problem

Adding a project, then removing its folder outside T3 Code, causes every enrichment retry to stat the missing root again. Missing-root favicon failures receive a zero resolver cache TTL instead of the existing negative cache window.

Change

Map only missing-workspace-root favicon failures to null inside the existing resolver cache. They receive the one-minute negative TTL; other errors retain their existing handling. Remove the obsolete missing-root logging exception from project enrichment. Reuse the existing regression test and implementation in this PR.

Scope and approval

Fixes https://github.com/pingdotgg/t3code/issues/23658

GitCafe: #23658

Triaged bug via the automated issue flow. This existing contributor PR is reused; no duplicate PR or implementation was created. Scope is server favicon caching only, with no contract or client changes.

Verification

Verified on fs-23, Linux, with the real web dev server and isolated worktree state. Main was fef851bee0. The after run used PR head c9b72ad409 merged locally with that main at 2bcbc9ab048fc56c8a5aa8ed7a98c861c64267c2; all three changed project files are identical to the PR head. The local merge was not pushed.

  • Red: applying only this PR's test delta to main gives 1 failed, 13 passed. The missing-root test fails with ProjectFaviconResolutionError caused by WorkspaceRootNotExistsError.
  • Green: vp test run src/project/ProjectFaviconResolver.test.ts src/project/ProjectEnrichmentService.test.ts from apps/server: 18 passed. The regression checks two null results and exactly one root stat.
  • Scoped lint on the three changed files passed. Server tsc --noEmit passed with no errors.
  • Two independent diff reviews (GPT-6.1 Sol and Claude Opus 5.5) found no confirmed findings.
Runtime proofBeforeAfter
Deleted project folder, cold caches, real browser shell requestsTranscript: uncached failures at 10:50:22, :30, :37 UTCTranscript: one initial uncached missing-root lookup, outer resolution succeeds; no repeats through three manual HTTP 200 requests at 10:53:54, 10:54:01, 10:54:16 UTC

Visual fixture matrix is empty: no render paths changed. Other asset/file errors for the deleted root remain outside this fix. Native desktop/mobile, remote/relay/tunnel connections and cache expiry/recovery were not exercised; the shared server path was verified through web. Missing roots now use the same existing 15-minute enrichment success TTL as other no-favicon results.

Evidence: red output, green output, lint, typecheck, browser runtime driver, test delta, request records.

Implementation: Claude Sonnet 5.5 (xhigh) via T3 Code. Triage and verification: GPT-6.1 Sol via Codex, Claude Opus 5.5 via Claude Code, in T3 Code.

Checks

21 successful · 8 skipped
All checks passed
dcd9fc6

Added 1 commit

  • fix(server): cache favicon lookups for missing project foldersc9b72ad
CcoderabbitaiBot19d ago
Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Missing workspace root errors during favicon resolution now return null and receive the cache’s negative TTL. Project enrichment sends these failures through the standard logging path. A regression test confirms that two resolutions stat the missing root once.

Changes: Favicon resolution

Layer / File(s)Summary
Missing-root handling and cache test
apps/server/src/project/ProjectFaviconResolver.ts, apps/server/src/project/ProjectEnrichmentService.ts, apps/server/src/project/ProjectFaviconResolver.test.ts		The cache loader converts missing-workspace-root errors to null. Enrichment no longer skips logging these failures. The regression test verifies that two resolutions stat the missing root once.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#28584: Invalidates the server favicon cache when a user saves a selected icon; its reported failure involved the negative-cache behavior changed here.

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to dcd9f

Missing-folder favicon lookups should be cached rather than repeatedly checking the folder. No merge-blocking risk is identified.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly describes the main change: caching favicon lookups for missing project folders.
Description checkPassedThe description covers the problem, change, scope and approval, and verification. It identifies the linked issue and reports focused test, lint, typecheck, and runtime results, including untested area…
Linked Issues checkPassedIssue [#23658] requires a missing workspace-root favicon lookup to remain cached for at least one minute. ProjectFaviconResolver converts only normalize-workspace failures caused by `WorkspaceRoot…
Out of Scope Changes checkPassedAll changes support issue [#23658]. The resolver change implements negative caching. The regression test verifies the required behavior and uses a fresh workspace-path layer for its filesystem overrid…
Full details: Description check

Explanation

The description covers the problem, change, scope and approval, and verification. It identifies the linked issue and reports focused test, lint, typecheck, and runtime results, including untested areas.

Full details: Linked Issues check

Explanation

Issue [#23658] requires a missing workspace-root favicon lookup to remain cached for at least one minute. ProjectFaviconResolver converts only normalize-workspace failures caused by WorkspaceRootNotExistsError to null. The existing cache assigns successful null results a one-minute TTL and keeps other failures at zero TTL. The regression test verifies two null results and one root stat. The enrichment-service cleanup removes the obsolete missing-root error check.

Full details: Out of Scope Changes check

Explanation

All changes support issue [#23658]. The resolver change implements negative caching. The regression test verifies the required behavior and uses a fresh workspace-path layer for its filesystem override. The enrichment-service change removes handling made obsolete by the resolver behavior. No unrelated change is demonstrated.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between c9b72ad4093a2eec877f882f30c5119ca70dee17 and dcd9fc6a8d1b7ec24e8dc0fe3fb02ec75b857dd8.

External image Files selected for processing: 2

  • apps/server/src/project/ProjectEnrichmentService.ts
  • apps/server/src/project/ProjectFaviconResolver.ts
External image Files with no reviewable changes: 1

  • apps/server/src/project/ProjectEnrichmentService.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Added 1 commit

  • Merge branch 'main' into fix/23658-favicon-missing-root-cachedcd9fc6