Add /favicon.ico route for connector icon#81
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Owner
Author
Owner
Author
|
Needs rebase after #80 merges. Holding until then. |
Anthropic's Connectors UI uses Google's favicon service to show connector icons. Serve the awareness logo at /favicon.ico from both middleware classes so it displays in Claude.ai instead of a generic globe. Route is public (no secret path required) so external crawlers can reach it. - favicon.ico copied into package, included in wheel via force-include - SecretPathMiddleware: serves before secret path check - HealthMiddleware: serves alongside /health - 2 new tests (15 total in test_middleware.py) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
CI green after rebase. Ready for QA. (Manual promotion — force push breaks workflow_run PR lookup.) |
cmeans
commented
Mar 28, 2026
Owner
Author
cmeans
left a comment
There was a problem hiding this comment.
QA Review — PR #81: Add /favicon.ico route for connector icon
Code Review
- Favicon loaded at import time (
_FAVICON_BYTES) — efficient, no repeated disk I/O. GracefulNonefallback if file missing. - Both middleware classes serve
/favicon.ico—SecretPathMiddlewareserves before secret check (correct for public route),HealthMiddlewareserves alongside/health. HealthMiddlewarerefactored to nestedifwithinscope["type"] == "http"— clean, no logic change for existing/healthbehavior.pyproject.tomlforce-include mirrors existinginstructions.mdpattern.- Response uses
image/x-iconcontent type — correct for.icofiles. - AGPL header present on modified
middleware.py(pre-existing from #80).
Test Results
| Check | Result |
|---|---|
test_middleware.py — 15 tests |
✅ (was 13, +2 favicon) |
| Full suite — 383 tests | ✅ (was 381, +2 favicon) |
| ruff | ✅ (via CI) |
| mypy | ✅ (via CI) |
| codecov/patch | ✅ |
Manual Tests
| # | Test | Result |
|---|---|---|
| 1 | Favicon served — HTTP 200, image/x-icon, 15,086 bytes | ✅ |
| 2 | Favicon served without secret path | ✅ (tested on HealthMiddleware instance) |
| 3 | Health endpoint still works — {"status":"ok"} |
✅ |
| 4 | MCP endpoint unaffected — get_briefing works |
✅ (production instance verified via awareness MCP) |
| 5 | test_middleware.py — 15/15 pass |
✅ |
Verdict
Zero findings. Clean, focused PR. All 5 manual tests pass, 383/383 automated tests pass. Ready for signoff.
Owner
Author
|
Adding Ready for QA Signoff — all 5 manual tests pass, 383/383 automated tests (15/15 middleware), CI green, zero findings. Removing Ready for QA. |
Merged
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.
Summary
/favicon.icofrom both ASGI middleware classesfavicon.icoincluded in the wheel viaforce-includeinpyproject.tomlQA
Prerequisites
pip install -e ".[dev]"AWARENESS_PORT=8421)Manual tests (via HTTP — middleware routes, not MCP tools)
Expected: HTTP 200, Content-Type: image/x-icon, ~15KB body
Expected: Returns 200 even though other non-secret paths return 404
Expected: JSON with status "ok"
Verify
get_briefingstill works via MCP clientExpected: 15 tests pass (was 13, +2 favicon tests)
🤖 Generated with Claude Code