Fix skybox console errors, add Puppeteer animation skills + Matterhorn demo, fix screenshot self-navigation - #3
Merged
Merged
Conversation
emirkaan5
commented
Aug 5, 2026
emirkaan5
commented
Contributor
- Fix skybox console errors on the web map.
- Puppeteer animation skills (reference examples) + a Matterhorn demo GIF in the README header.
- Headless-screenshot loopback fix so screenshots work when MAPCONTROL_PUBLIC_URL isn't reachable from inside the container — and revert of an image-default port change that would have broken existing deployments.
Reference-only skills showing how to drive the web map with Puppeteer for different animation scenarios: ballistic flyTo tours, 3D terrain orbit (Matterhorn), keyframe screenshots, and frame-sequence recording. Each scenario ships a SKILL.md recipe plus a runnable animate.mjs, over a shared lib/esip-map.mjs helper that targets the page's published MapLibre instance. Wired links into the README (docs table + Demos section). Terrain examples use map.setSky(), reinforcing the correct MapLibre API. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vigation
Reverts the image's default-port change (8000→8080) so existing deployments
that reference 8000 in Compose, proxies, MAPCONTROL_PUBLIC_URL, or SDK calls
keep working. A host-side port conflict is handled the normal Docker way —
remap the host port and leave the container port unchanged ("8080:8000").
Screenshot self-navigation fix: take_screenshot built the URL Chromium opens
from _public_base_url() (MAPCONTROL_PUBLIC_URL). When that is an external
address or reverse-proxy origin, it does not resolve back to this process from
inside the container, so every screenshot failed. Chromium now self-navigates
via the new _internal_base_url() — loopback on the server's bound port — while
_public_base_url() stays in use for URLs returned to external clients. Applied
at both call sites: the MCP take_screenshot tool (mcp_tools.py) and the
Playwright fallback (main.py).
The bound port and config must agree for loopback to work: the working-dir
config.toml carries a dev port (7777) while the image binds 8000, so the
Dockerfile now sets ENV MAPCONTROL_PORT=8000 (config.py already lets that
override config.toml) — making load_config().server.port == the bound port in
the container without changing the image default.
Also:
- regression test (server/tests/test_screenshot_loopback.py): with an
unreachable MAPCONTROL_PUBLIC_URL and the container's port split, asserts
take_screenshot self-navigates to loopback:8000, not the public host or the
config.toml dev port.
- keep the .gitignore *.log entry.
- README: note the "8080:8000" host-remap for when host port 8000 is occupied.
- puppeteer-skills docs: sync example URLs back to :8000.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
isConic
approved these changes
Aug 6, 2026
isConic
left a comment
isConic
left a comment
Contributor
There was a problem hiding this comment.
Excellent resubmit — this is exactly what was asked for, plus a catch I didn't spec. Approving.
What I verified:
- setSky fix — intact and unchanged from the original PR:
enableSky()/disableSky()replacing the four Mapbox-styleaddLayer({type:'sky'})blocks. Correct MapLibre 5+ API, nice dedup. - Port sweep dropped — net diff is clean; the image contract stays on 8000. The README troubleshooting note (host-side
8080:8000remap) is the right way to serve Mac users with a port clash. - Loopback self-navigation fix —
_internal_base_url()inmcp_tools.py, used by both call sites (the MCPtake_screenshottool and the Playwright fallback inmain.py). Public URLs stay public, self-navigation stays loopback. This is the root-cause fix. - Bonus catch:
ENV MAPCONTROL_PORT=8000in the Dockerfile — good spot. Without it,load_config().server.portwould read the working-dir config.toml dev port (7777) and the loopback URL would target the wrong port inside the container. The keep-in-sync comment next to CMD is appreciated. - Regression test —
test_screenshot_loopback.pypins the invariant hard: unreachable public URL, asserts nav URL is127.0.0.1:<bound port>, and explicitly checks that neither the public host nor the 7777 dev port leaks. Runnable standalone and via pytest, matching the other gates. - Puppeteer skills + Matterhorn GIF — additive reference docs, no runtime impact. Nice showcase for the README header.
- CI on the head commit is green (
build-test-pushsuccess).
Two follow-ups (non-blocking, happy to take either as a separate PR):
- Wire
test_screenshot_loopback.pyintopublish-image.ymlas a gate alongsidetest_mcp/test_mcp_auth/test_portal, so the invariant is enforced on every push. main.pyimporting the underscore-private_internal_base_urlfrommcp_toolsworks but reads as private-API reach-in — consider promoting it to a public name later.
Squash-merging to collapse the port-change/revert pair in the branch history into one clean commit. Thanks Emir — two real bug fixes and a demo upgrade in one PR. 🏔️
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
2 participants
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.