Imported from wilmtang/better-peakbagger (
AGENTS.md). Install upstream withnpx 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.jsis 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 puresrc/capture/capture-core.jsmodule rather than coupling those algorithms to browser APIs. Geometry and gain primitives shared with the GPX Analyzer live in the puresrc/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.jsbundle. Chrome'sbackground.service_workerand Firefox'sbackground.scriptsmust both name that same output; its ES-module composition belongs only in thebackground.jsentry inscripts/build-config.mjs. Do not reintroduceimportScriptsor raw source arrays.test/project/manifest-capture.test.mjspins 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.jsand the on-demandsrc/capture/provider-page.jsrun 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.jsis the narrow settings bridge from the isolated world to the MAIN-world GPX Analyzer viawindow.postMessage.src/settings/settings.jsownschrome.storage.syncaccess only; the schema itself — defaults, bounds, validators — lives in the puresrc/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 arrangementsrc/gpx/gpx-metrics.jsuses 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 throughsrc/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 whateverclean()writes the readers accept unchanged.test/settings/settings-schema.test.mjspins 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.jsmust 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 aftersrc/ascent/ascent-draft.jsand 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.jsapplies the theme atdocument_startusing a synchronous page-local mirror to avoid a light-mode flash, then reconciles withstorage.sync; preserve the stylesheet-before-theme invariant when changing theme startup. - Page features stay separated by surface:
src/gpx/gpx-analyzer.jsowns ascent GPX analysis, map synchronization, and its extension-owned route overlay;src/ascent/ascent-filter.jsowns PeakAscents filtering and in-DOM sorting; andsrc/ascent/ascent-draft.jsowns 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 (seedocs/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 testbuilds 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:verifyrenders 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:chromeis the only single-browser check that loads the real unpackeddist/, 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 broadernpm run verify:browsers) after touchingmanifest.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-extensionoutright, so point at Chrome for Testing (npx playwright install chromium). Playwright's defaultheadless: truelauncheschrome-headless-shell, a separate binary with no extension support at all; usechannel: 'chromium'withheadless: trueto get full Chrome for Testing in new headless.scripts/verify-extension.mjsis 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/listproves 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-gputo 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-gpustays 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.jsrefuses any URL whose protocol is nothttps: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 disabledterrain:verify,terrain:verify:firefox, andshowcase: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-rulesornetwork.dns.localDomains, accept the certificate for that launch only, and delete the key and certificate in teardown.test/project/showcase.test.mjspins 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.
