Batch the workspace favicon probe (#1333, #1334)

[devswha]

The branch is ready for the board to push and open the PR. With the login fixed, gh can now read both issues. Neither has comments and no open PR covers this, so the fix didn't change.

While re-checking, I found main had moved by one commit (#1338), which conflicted with my branch on one import line in server/index.ts. #1338 had trimmed that import and I had added ProjectFavicon to it, so I kept main's version and added ProjectFavicon back. That's in merge commit 9c9f8bd, after the fix commit b635eb3, which closes #1333 and #1334.

After the merge, typecheck and build pass, and so do the favicon, access, proxy and conventions tests on the isolated herdr session. bun run check fast ends the same way as before: two unit test files fail, scripts/install-star.test.ts and server/portal.test.ts. The #1338 commit on main reports the same two failures on this Mac.

I didn't push or open the PR myself, because my task rules leave that to the board. The closing note is updated with the merge.

Also noticed

• src/components/Sidebar.tsx / server/workspace-routes.ts: /api/worktree/list logs one 404 per non-git workspace on each page load (herdr refuses it with not_git_worktree).
• scripts/install-star.test.ts: fails on a Mac with a display, on main too.
• server/portal.test.ts: fails during the full unit run and passes on its own, so it may be flaky.

Task: HWU-1 (herdr-web-ui/1)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: c210ea47-7f71-4176-afaf-3b3e53a399a0

📥 Commits

Reviewing files that changed from the base of the PR and between b5cdbdb and d7aad54.

📒 Files selected for processing: 13

• changelog.d/workspace-favicon-one-request.md
• scripts/file-viewer-regression.ts
• server/index.ts
• server/machine-api.ts
• server/machine-proxy.test.ts
• server/project-favicon.test.ts
• server/project-favicon.ts
• server/route-envelope.contract.test.ts
• shared/protocol.ts
• src/components/WorkspaceFavicon.tsx
• src/lib/api.ts
• src/lib/projectFavicon.test.ts
• src/lib/projectFavicon.ts

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

📝 Summary

Summary by CodeRabbit

• New Features
  • Workspace icons are now discovered through a single request per folder, including folders on remote machines. Folders without an icon return a normal empty result.
• Bug Fixes
  • Requests that receive rate-limit or server errors can be retried when the workspace row appears again; successful results and eligible client errors are cached.

The server now returns a project favicon in one request per folder, or a null icon when no candidate image is found. The client probes by machine and folder, caches responses, and retries after network errors, server errors, and 429 responses.

Changes: Workspace favicon request flow

Layer / File(s)	Summary
Project favicon response and lookup shared/protocol.ts, server/project-favicon.ts, server/index.ts, server/project-favicon.test.ts, server/route-envelope.contract.test.ts	The server checks ordered image candidates and returns their file details or a null icon. The route validates the method and root and applies the documented access checks. Tests cover candidate selection and route responses.
Machine-proxied favicon requests server/machine-api.ts, server/machine-proxy.test.ts	The machine proxy accepts favicon requests. Tests cover the response, bridge authorization, cookie handling, and cross-site rejection.
Client request, cache, and workspace display src/lib/api.ts, src/lib/projectFavicon.ts, src/lib/projectFavicon.test.ts, src/components/WorkspaceFavicon.tsx, scripts/file-viewer-regression.ts, changelog.d/workspace-favicon-one-request.md	The client requests one favicon response per machine and folder. It shares concurrent probes, caches successful answers and selected refusals, and retries network errors, server errors, and 429 responses. The workspace component and regression filter use the favicon endpoint. The changelog describes the request behavior.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: nahwan-kim

Merge Risk: ⚪ Minimal · up to d7aad

The favicon batching change is mergeable after normal checks; no actionable risk remains established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description includes validation results and issue references, but it does not provide the required Change section or clearly describe the problem and resulting behavior. It mainly documents branch…	Add a Change section that explains the repeated favicon requests, the new one-request-per-PC-and-folder behavior, null responses for missing icons, and retry behavior. Add a Validation section that lists the relevant commands and explicitly…

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the main change: batching workspace favicon probes.
Linked Issues check		For directly linked issue #1333, the client now makes one fetchProjectFavicon request for each machine-and-root key instead of requesting every candidate through /api/fs/stat. `server/project-favi…
Out of Scope Changes check		The changed endpoint, machine-proxy allowlist, server resolver, client cache, regression-test filter, protocol type, and related tests directly support the favicon request-batching fix for #1333. The …
Docstring Coverage		Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 12 files. (1 skipped: 1…

Full details: Linked Issues check

Explanation

For directly linked issue #1333, the client now makes one fetchProjectFavicon request for each machine-and-root key instead of requesting every candidate through /api/fs/stat. server/project-favicon.ts checks the ordered candidates behind one /api/fs/favicon request and returns null for a miss. src/lib/projectFavicon.ts caches successful answers, including null, and removes failed or 429 probes so a later request can retry. The supplied summary reports tests for deduplication, cache separation, retries, route access, and proxy behavior.

Full details: Out of Scope Changes check

Explanation

The changed endpoint, machine-proxy allowlist, server resolver, client cache, regression-test filter, protocol type, and related tests directly support the favicon request-batching fix for #1333. The changelog documents that behavior. The reported /api/worktree/list 404s are pre-existing behavior and are not introduced as changes in this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 12 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description includes validation results and issue references, but it does not provide the required Change section or clearly describe the problem and resulting behavior. It mainly documents branch and merge activity.

Resolution

Add a Change section that explains the repeated favicon requests, the new one-request-per-PC-and-folder behavior, null responses for missing icons, and retry behavior. Add a Validation section that lists the relevant commands and explicitly names the HTTP contract-test coverage.

• Fix all pre-merge checks with AI

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Review 1 · Pass · 1 pre-existing

Does the change do what the task asked? Yes. Gate: major and worse must be fixed.

The favicon lookup now takes one request per PC and folder, returns ordinary misses without a 404, and keeps candidate ordering and per-session caching behavior. The server tests exercise the local and remote routes plus access restrictions, and the client tests cover deduplication and retry after failed checks. One separate preexisting worktree classification request can still log a 404 for each non-git workspace on page load.

Not required (there before this change)

• trivial: Sidebar worktree classification still produces a separate 404 for each unmarked non-git workspace on page load. (src/components/Sidebar.tsx:209; server/workspace-routes.ts:133)
  Proof: Inspection showed Sidebar requests listWorktrees for every unmarked workspace; the handler returns the herdr error response, and the client caches 4xx refusals as a non-repository result. This is separate from the favicon candidate burst and existed before this change.

[devswha]

Review 3 · Pass · 1 pre-existing

Does the change do what the task asked? Yes. Gate: major and worse must be fixed.

The change meets the favicon batching requirements: the browser uses one request per PC and folder, misses return 200 with a null icon, and failed checks are not cached. I verified the endpoint and client tests, type generation, typecheck, and observed a page load issue one favicon request for each workspace. A separate worktree-list 404 remains as a preexisting single-request-per-workspace behavior, rather than the favicon candidate burst.

Not required (there before this change)

• trivial: The sidebar still requests /api/worktree/list for every unmarked workspace, and non-git workspaces return a 404 on page load. (src/components/Sidebar.tsx: repository workspace detection; server/workspace-routes.ts: /api/worktree/list)
  Proof: Confirmed the Promise.all listWorktrees call and 4xx miss handling are present unchanged in the base commit c4c842b. This is a single capability check per workspace, not the 33-request favicon probe burst.

[devswha]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.