better-peakbagger

Imported from wilmtang/better-peakbagger (AGENTS.md). Install upstream with npx skills add wilmtang/better-peakbagger. Copyright stays with the author.

Repository agent instructions

Act like a skeptical senior engineer. Inspect relevant code before editing, identify root causes and broken invariants, and prefer the smallest safe change. Add regression tests when practical, state exactly what was verified, and do not perform unrelated refactors.

When making code changes, commit each completed independent unit of work before starting another. Keep every commit focused and do not bundle unrelated changes. Run checks appropriate to the change before committing; do not commit knowingly broken or incomplete work merely to satisfy this rule. Preserve unrelated user-owned working-tree changes unless the user explicitly asks to include them. Codex commits must follow the explanatory Claude Code style exemplified by 0d5c3fe: use a lowercase Conventional Commit subject — <type>: <concise lowercase summary> — and add a body only when it records context, constraints, or verification the subject can't convey. When a body is warranted, identify the concrete module-level changes, call out intentionally preserved exceptions or boundaries, and end with the checks actually run and their results. Pass multi-paragraph bodies with repeated -m arguments or -F; never embed literal \n escapes in a shell argument, and inspect git log -1 --format=raw before moving on. A parenthesized step label is appropriate only when the work is part of an explicitly numbered migration or plan. Do not claim verification that did not occur.

For audit-remediation work, keep an explicit closure ledger with three categories: fixed and verified, intentionally not changed, and changed but not fully proven. Do not summarize the audit as completely fixed while owner-choice items or verification gaps remain. Preserve that ledger when archiving the completed plan so the handoff distinguishes product risk from optional cleanup and missing evidence.

Architecture

This is a Manifest V3 browser extension for Chrome and Firefox. Runtime source is authored as ES modules and bundled with esbuild into dist/; load, verify, lint, and package dist/, never the repository root. manifest.json is the source of truth for permissions, execution worlds, and the order of separately loaded bundles/vendor scripts. scripts/build-config.mjs is the source of truth for each bundle's module composition and copied assets. See docs/development.md for the workflow, docs/architecture.md for the maintained design overview, and PRIVACY.md for the public data-handling contract; keep this section focused on boundaries that code changes must preserve.

For interactive browser work, npm run start -- chromium and npm run start -- firefox own esbuild watch mode and reload the extension only after a complete successful build. Do not run either command beside npm run watch or another build in the same worktree. The extension reload does not reinject content scripts into an already-open site tab; refresh that tab manually when its scripts change.

• src/background/background.js is the service-worker coordinator for activity-capture jobs, Peakbagger summit lookup, temporary session state, draft tabs, and handshakes. Keep reusable track validation, scoring, metrics, and GPX reduction in the pure src/capture/capture-core.js module rather than coupling those algorithms to browser APIs. Geometry and gain primitives shared with the GPX Analyzer live in the pure src/gpx/gpx-metrics.js, bundled into both the background worker and the MAIN-world analyzer; shared math must stay there so drafted and displayed values cannot diverge.
• The worker ships as one dist/background.js bundle. Chrome's background.service_worker and Firefox's background.scripts must both name that same output; its ES-module composition belongs only in the background.js entry in scripts/build-config.mjs. Do not reintroduce importScripts or raw source arrays. test/project/manifest-capture.test.mjs pins the two manifest references, bundle composition, and a real bundled-worker boot.
• Peakbagger content scripts normally run in the isolated extension world, where extension APIs are available. src/gpx/gpx-analyzer.js and the on-demand src/capture/provider-page.js run in the page's MAIN world because they need page-owned globals or authenticated same-origin page state; MAIN-world code cannot call extension APIs. Do not move code across this boundary without re-evaluating its data access and browser compatibility.
• src/settings/bridge.js is the narrow settings bridge from the isolated world to the MAIN-world GPX Analyzer via window.postMessage. src/settings/settings.js owns chrome.storage.sync access only; the schema itself — defaults, bounds, validators — lives in the pure src/settings/settings-schema.js, which has no DOM or extension-API dependency and is imported into the MAIN-world and terrain-frame bundles too (the same arrangement src/gpx/gpx-metrics.js uses for shared math). Page-world writes are allowlisted to the analyzer-owned keys only; feature gates, capture privacy options, and theme stay writable solely from extension surfaces. Do not widen the allowlist, create a second settings schema, or expose privileged extension messaging directly to page-world code.
• Every surface that reads settings off postMessage — the GPX Analyzer, the BigMap enhancer, the terrain frame — must re-validate, because that message crosses a trust boundary. Re-validate through src/settings/settings-schema.js, never with a local copy of a bound or a default: those copies are what "do not create a second settings schema" forbids, and a drifting bound is invisible until it ships. Validation is idempotent by design, so whatever clean() writes the readers accept unchanged. test/settings/settings-schema.test.mjs pins the shared validation semantics and scans for the route, viewport, and theme literals that previously drifted; extend that structural guard whenever a new shared default or bound is added.
• Activity capture is an explicit, short-lived transaction started by a toolbar click and scoped by activeTab; there are intentionally no persistent Garmin or Strava host permissions. src/capture/provider-page.js must verify ownership before fetching provider GPX and must fail closed when ownership signals or provider DOM are ambiguous.
• Raw provider GPX is parsed on the activity page and must never leave that page or be persisted. Analysis fields may reach the background worker; capture settings may additionally allowlist the activity/track name for multi-peak Trip Info and waypoint latitude/longitude/name. The later Peakbagger Preview payload must be newly serialized from these narrow fields—never forward or redact the source XML in place. Waypoints are on by default, consume the same 3,000-point budget as trackpoints, and must not carry elevation, time, description, symbols, or extensions.
• Summit lookup must be complete before results are presented: partial corridor responses are not equivalent to "no peaks." Privacy or correctness gates—including Peakbagger login, ownership, track validation, draft identity, and expected form structure—must remain fail closed.
• Prepared drafts live in storage.session, expire after 30 minutes, and are delivered only after src/ascent/ascent-draft.js and the background worker verify the sender tab plus job, peak, and climber identity. Draft filling may trigger GPS Preview exactly once, but no extension path may click either Peakbagger Save control; final review and Save always belong to the user.
• Assign Peakbagger's alphabetical suffixes only among selected drafts that share an ascent date, using track-encounter order before confidence-ranked tab opening; singleton dates keep the suffix blank. Encounter time is not a Peakbagger suffix and must not be written into SuffixText. Multi-peak trip names prefer the first GPX track name, then the activity page heading, then selected summit names joined in track order; normalize whitespace in GPX/page names and limit every result to 200 characters.
• Site settings and theme originate in src/settings/settings.js. src/theme/theme.js applies the theme at document_start using a synchronous page-local mirror to avoid a light-mode flash, then reconciles with storage.sync; preserve the stylesheet-before-theme invariant when changing theme startup.
• Page features stay separated by surface: src/gpx/gpx-analyzer.js owns ascent GPX analysis, map synchronization, and its extension-owned route overlay; src/ascent/ascent-filter.js owns PeakAscents filtering and in-DOM sorting; and src/ascent/ascent-draft.js owns validated draft filling. The route overlay must not mutate Peakbagger's native layers and must remain behind its native route and markers. Prefer extending the owning surface over adding cross-feature globals. Chart clock times and day boundaries are the climb's local time, resolved offline from the track's starting coordinate via the npm-sourced, packaged tz-lookup raster; timezone failures must fall back to the labelled longitude estimate, never break the panel, and never send coordinates off the page (see docs/mountain-local-time.md).
• Tests mirror these boundaries under test/: pure capture algorithms, provider adapters, background handshakes, draft privacy/exactly-once behavior, popup behavior, settings/theme, and fixture-based Peakbagger pages. Add focused regression coverage beside the affected boundary; live provider DOM and export behavior still require manual browser verification before release.
• Know what each check cannot see, because a green run has already shipped a dead extension. npm test builds and evaluates shipped IIFE bundles in jsdom, but it never lets a browser interpret the real manifest: execution worlds, separately loaded script order, and the live worker lifecycle remain invisible. npm run terrain:verify renders the true MapLibre frame, but its showcase pages provide their own storage and bridge-protocol stubs, so it never exercises the real cross-world bridge. npm run verify:chrome is the only single-browser check that loads the real unpacked dist/, and the only one that can catch a broken manifest reference/order, a bailed-out service worker, or a silent bridge. Run it (or the broader npm run verify:browsers) after touching manifest.json, scripts/build-config.mjs, execution worlds, the worker, or anything a content script depends on at load.

Real-browser verification — do not interrupt the user (must follow)

Real-browser checks must not steal focus, switch the user's Space, cover the user's working display, or reuse the user's normal browser window/profile:

• Prefer fixtures and background browser control first. Use a hidden/headless/offscreen test profile plus CDP, WebDriver BiDi, or the browser's debugging protocol for DOM inspection, synthetic input, and page/window screenshots whenever that can verify the behavior. Capture the target page or window, never the whole display. Do not use Computer Use against the user's existing browser for routine verification when an isolated protocol-driven session can do the job.
• Hidden verification does not prove native focus, window placement, browser chrome, menus, permission prompts, or other onscreen behavior. If one of those is the behavior under test, a visible window is allowed only in a dedicated test profile. When multiple displays are attached, place the entire test window on the built-in display before navigating or interacting and leave the external working display untouched. Avoid activating or raising the test window where the platform permits background launch and control.
• Loading the real unpacked extension headlessly works, but only one combination does it, and both failure modes look like "extensions need a visible window" if you stop at the first one. Chrome stable 137+ refuses --load-extension outright, so point at Chrome for Testing (npx playwright install chromium). Playwright's default headless: true launches chrome-headless-shell, a separate binary with no extension support at all; use channel: 'chromium' with headless: true to get full Chrome for Testing in new headless. scripts/verify-extension.mjs is the worked example. Also note an MV3 service worker is lazy: it does not appear as a CDP target until an event wakes it, so its absence from /json/list proves nothing — probe for content-script injection, or send it a message, instead of inferring from an empty target list.
• Graphics checks run on the GPU, headless. Headless Chrome reaches the real hardware renderer (Metal on macOS, the platform default elsewhere), so a hidden run and a GPU run are not a trade-off—do not reach for a visible window to get hardware GL. Never pass --use-angle=swiftshader, --enable-unsafe-swiftshader, or --disable-gpu to anything rendering WebGL: SwiftShader software-renders MapLibre's terrain into minutes of pegged CPU, caps textures at 8192, and proves nothing about the renderer users have. Assert the renderer (WEBGL_debug_renderer_info) and fail closed on a software fallback rather than trusting the flags, because software output still looks plausible in a screenshot. --disable-gpu stays acceptable only for static, non-WebGL page screenshots where it buys determinism. If a graphics check is burning CPU, suspect the renderer before the workload.
• Keep live Peakbagger checks minimal, read-only, and rate-limited; repeatable coverage belongs in the masked fixtures. If generic Selenium or Playwright is challenged by Cloudflare, follow peakbagger-cli's browser transport: use Patchright with installed Chrome in an isolated persistent test profile, run headful only because the challenge requires it, wait for clearance, and reuse only the clearance cookies with the exact User-Agent that minted them. Keep this compatibility mechanism in test tooling—never extension runtime code—and do not weaken rate limits, mass-scrape, or automate a CAPTCHA.
• Batch visual checks into as few launches as practical. Wrap every browser/profile/debug-server launch in reliable teardown so pages, contexts, browsers, and helper processes close even after failures. Remove disposable profiles and artifacts unless they are an explicitly managed test cache. A successful verifier exit is not proof of teardown: before handing off, inspect the remaining process command lines and exact disposable profile paths, then remove only confirmed test artifacts without disturbing the user's browsers or reusable tooling.
• Gate on the condition, never on a fixed sleep, and let the fixture's timing match a real user's. A delay() long enough on an idle machine is not long enough on a loaded one, and the resulting failure reads as a product bug rather than a slow tick; poll with a timeout and report the live value at failure time, not a snapshot taken earlier. Wait for the user-visible postcondition, not merely an outgoing message or storage mutation that can precede the UI's final state. Fixtures that drive the UI must wait for what the feature actually reads — the terrain check flaked for exactly this reason, auto-clicking 3D as soon as the GPX parsed while the drape is read from the separately-loading MasterMap frame, producing a correct "Terrain only" fallback that looked like a regression. Treat a check that passes on re-run as a bug in the check, not as noise.
• Serve every browser fixture over HTTPS on a real Peakbagger hostname, not http://localhost. src/peakbagger/peakbagger-request.js refuses any URL whose protocol is not https: or whose host is not Peakbagger's, and product code fetches through that guard, so a plain-HTTP fixture makes the extension refuse its own fixture and the check fails for a reason that has nothing to do with the behavior under test. This is not hypothetical: it silently disabled terrain:verify, terrain:verify:firefox, and showcase:render — the last of which then rendered "Better Peakbagger refused an invalid Peakbagger request." straight into the store-listing screenshots. Mint a disposable self-signed certificate per run (openssl req -x509 …), point the browser at the host with --host-resolver-rules or network.dns.localDomains, accept the certificate for that launch only, and delete the key and certificate in teardown. test/project/showcase.test.mjs pins this for every fixture-serving script.
• State whether verification ran hidden or visible, what renderer/browser and viewport were used, and any onscreen behavior the chosen method could not establish. A process check, DOM assertion, or protocol screenshot is not evidence for focus or window-placement behavior.

UX bar — design like a senior Apple designer

Hold every user-facing change to the clarity, restraint, and finish a senior Apple product designer would expect. Optimize for the user's outcome, not for exposing implementation machinery, and never trap or surprise the user.

• Clarity first. Give each surface one obvious primary action. Use plain language, explain confidence and failure states in terms users can act on, and move technical detail to the README instead of crowding the popup.
• Restraint. Prefer a sensible default over another setting, control, banner, or line of copy. Reuse the extension's existing components and visual language; Strong and Probable states must stay consistent everywhere.
• Reversible and safe. Make consequential actions explicit, keep temporary feedback dismissible, preserve manual review before Save, and never transmit or retain activity data beyond the consent and privacy boundaries documented by the product.
• Native browser feel. Use familiar browser and platform affordances, keyboard navigation, meaningful focus states, accessible names, sufficient contrast, and layouts that work in both Chrome and Firefox, light and dark contexts.
• Show, don't tell. Prefer a concise visual cue, progress state, or actionable error over explanatory prose. Motion should be brief, purposeful, and respect reduced-motion preferences.

For UI changes, add focused behavior tests when practical and visually inspect the real rendered result at relevant popup/page sizes before calling the work complete. Confirm before implementation that the intended verification surface can load the target extension or fixture. If policy or tooling blocks the render, report the exact page state and viewport that remain uninspected; extension startup, DOM assertions, and interaction tests prove behavior, not spacing, wrapping, clipping, or visual polish.