[feature/satellite] OMM data download, server background driven update, optional Space-Track

[MichaelWheeley]

What does this PR do?

• obsolete satellite TLE download replaced with OMM throughout stack
• OMM download for groups using CSV for optimum efficiency, or JSON for individual downloads
• data download no longer event driven but rather happens in background on server using protected state-machine
• improved detection and lockout of CelesTrak 403, touchwood no more getting kicked off their server
• add Space-Track as optional OMM source, controlled from .env - requires login and cookie support
• add ability to disable CelesTrak as source, .env.example modified with details
  • possible to disable both sources if satellite data not wanted at all
• AMSAT and SatNOGS dropped as providers, not OMM providers and data questionable
• add list of satellites whose data sets are known but which are discarded, see /api/satellites/debug
• end point change /api/satellites/data (was /api/satellites/tle)
• kill React warning in debug log for useSatelliteLayer wheel = { passive: true }

Type of change

• Bug fix
• New feature
• Performance improvement
• Refactor / code cleanup
• Documentation
• Translation
• Map layer plugin

How to test

1. see the .env.example on how to enable / disable CelesTrak, and Space-Track with a username/password
2. LOG_LEVEL=debug for additional messaging
3. /api/satellites/data and /api/satellites/debug for status

Checklist

• App loads without console errors
• Tested in Dark, Light, and Retro themes
• Responsive at different screen sizes (desktop + mobile)
• If touching server.js: caches have TTLs and size caps (we serve 2,000+ concurrent users)
• If adding an API route: includes caching and error handling
• If adding a panel: wired into Modern, Classic, and Dockable layouts
• No hardcoded colors — uses CSS variables (var(--accent-cyan), etc.)
• No .bak, .old, console.log debug lines, or test scripts included

[accius]

Thanks for taking this on, Michael — moving the satellite refresh to a background state machine and switching to OMM is a solid direction, and the CelesTrak 403 lockout handling is a clear improvement. CI is green and the diff is clean. A few things I'd like to see addressed before this merges, mostly around the network fetch correctness and a couple of latent bugs.

Blocking-ish / correctness

1. axios abort signals are passed in the wrong argument slot. In fetchOmmFromSpaceTrack, client.post(url, body, controller.signal) passes the signal as the config object, and client.get(url, {...}, controller.signal) passes it as a fourth arg that axios ignores. axios expects { signal } inside the config object. As written the 20s AbortController timeouts never actually abort the Space-Track login/whoami/fetch calls. That matters a lot here: the state machine serializes run() through a mutex, so a single hung Space-Track request with no working timeout will stall the entire background updater indefinitely (CelesTrak included). Please pass { ..., signal: controller.signal } in the config object and verify abort actually fires.

2. Two implicit global leaks. In isLoggedIn, jsonBytes = new Uint8Array(whoami) and in appendDataToOmmCache, match = knownNoradIds.has(noradId) are both undeclared (no const/let). There's no 'use strict' in this file, so these become properties on the global object and are shared/overwritten across calls. Add declarations.

3. celestrakSatsToDownload does s.data_source.startsWith('celestrak') with no guard, but satellites-tracked.js's own header comment explicitly says data_source may be absent. Any satellite without that field will throw a TypeError inside the START handler. Guard with s.data_source?.startsWith('celestrak').

Error handling

4. The try { ... } finally { return 'NEXT_STATE'; } pattern in the handlers swallows thrown exceptions — a return inside finally discards any in-flight throw. If a handler hits an unexpected error you'll silently advance state with no log. Prefer an explicit catch that logs, then return the next state.

5. 429 is no longer treated as a rate-limit signal. The CelesTrak fetch handlers only trip the backoff on 301 || 403; the old code keyed on 429. CelesTrak does return 429 under load — please include it in the block condition so we keep the "no more getting kicked off their server" property the PR description claims.

6. appendDataToOmmCache does ommJson.forEach(...), which assumes an array. parseCsvText returns null on a parse failure, and the typeof ommJson === 'object' guard also lets non-array objects through. A non-array would throw. Tighten the guard to Array.isArray(ommJson).

Caching

7. ommUnusedCache has no size cap or TTL. Every unknown NORAD ID seen in the amateur/weather group CSVs gets recorded, and those groups are large. It's bounded by the group sizes so it won't grow without limit, but per the repo checklist ("caches have TTLs and size caps — we serve 2,000+ concurrent users") it's worth either capping it or noting why it's safe. /api/satellites/debug exposing the full unused list is fine for debugging, just be aware it's now a non-trivial payload.

8. Minor: the Space-Track login spoofs User-Agent: Mozilla/5.0, and the login POST response status is never checked — auth success is inferred only from the later whoami call. That works, but checking the login response too would fail faster and more clearly. Credentials themselves look handled correctly: kept server-side under _username/_password, never logged, sent in the form body — good.

Smaller notes

• noradsToDownload is a module-level variable reused as both an array (group/Space-Track paths) and a scalar (individual path). The mutex serializes states so it's safe today, but it's fragile; the thrown guard in CELESTRAK_INDIVIDUAL_FETCH throws a bare string instead of an Error.
• convert-csv-to-json is a fairly lightly-maintained dependency for what is essentially CSV-of-known-shape parsing — not blocking, just flagging the supply-chain footprint.
• Nice catch on the wheel { passive: true } warning fix.

Happy to re-review once the signal-passing and the two implicit globals are sorted — those are the ones I'd genuinely worry about in production.

— K0CJH

[MichaelWheeley]

Blocking-ish / correctness

1. axios abort signals are passed in the wrong argument slot. In fetchOmmFromSpaceTrack, client.post(url, body, controller.signal) passes the signal as the config object, and client.get(url, {...}, controller.signal) passes it as a fourth arg that axios ignores. axios expects { signal } inside the config object. As written the 20s AbortController timeouts never actually abort the Space-Track login/whoami/fetch calls......... Please pass { ..., signal: controller.signal } in the config object and verify abort actually fires.

• manually confirmed that AbortSignal thrown if timeout occurs in each of
  • const loginResponse = await client.post(...)
  • const whoamiResp = await client.get(...)
  • const res = await client.get(...)
• a couple of other details were also corrected,
  • there is no such function as logError - changed to logWarn, it was masked by outer exception handler
  • isCelestrakEnabled was not being used, when Space-Track did not return results CelesTrak was an active backup whether enabled or not

2. Two implicit global leaks. In isLoggedIn, jsonBytes = new Uint8Array(whoami) and in appendDataToOmmCache, match = knownNoradIds.has(noradId) are both undeclared (no const/let). There's no 'use strict' in this file, so these become properties on the global object and are shared/overwritten across calls. Add declarations.

• added 'use strict';
• added const to jsonBytes
• added const to match

3. celestrakSatsToDownload does s.data_source.startsWith('celestrak') with no guard, but satellites-tracked.js's own header comment explicitly says data_source may be absent. Any satellite without that field will throw a TypeError inside the START handler. Guard with s.data_source?.startsWith('celestrak').

• protect s.data_source.startsWith with s.data_source?.startsWith, noting that the other two uses of s.data_source === are already protected as they return false if data_source is undefined

Error handling

4. The try { ... } finally { return 'NEXT_STATE'; } pattern in the handlers swallows thrown exceptions — a return inside finally discards any in-flight throw. If a handler hits an unexpected error you'll silently advance state with no log. Prefer an explicit catch that logs, then return the next state.

• added explict messages on try{}catch{ logWarn(...); }

5. 429 is no longer treated as a rate-limit signal. The CelesTrak fetch handlers only trip the backoff on 301 || 403; the old code keyed on 429. CelesTrak does return 429 under load — please include it in the block condition so we keep the "no more getting kicked off their server" property the PR description claims.

• Added 429 as HTTP status code rate-limit detect

6. appendDataToOmmCache does ommJson.forEach(...), which assumes an array. parseCsvText returns null on a parse failure, and the typeof ommJson === 'object' guard also lets non-array objects through. A non-array would throw. Tighten the guard to Array.isArray(ommJson)

• protected function by replacement of typeof ... object with Array.isArray(...)

Caching

7. ommUnusedCache has no size cap or TTL. Every unknown NORAD ID seen in the amateur/weather group CSVs gets recorded, and those groups are large. It's bounded by the group sizes so it won't grow without limit, but per the repo checklist ("caches have TTLs and size caps — we serve 2,000+ concurrent users") it's worth either capping it or noting why it's safe. /api/satellites/debug exposing the full unused list is fine for debugging, just be aware it's now a non-trivial payload.

• CSV converted to ommJson for group downloads (CelesTrak or Space-Track), JSON directly downloaded as ommJson for individual downloads
• appendDataToOmmCache(ommJson) caches data only those satellites that are in the target list HAM_SATELLITES, ommJson is then discarded. The size of ommCache should not grow beyond the size of the target list.
• separate ommUnusedCache is much abbreviated, it has fields for only name and norad. The size of ommUnusedCache should not grow beyond the intersection size of the groups amateur/weather minus the intersection of the target list.
• remarks added to this effect, but no code changes made

8. Minor: the Space-Track login spoofs User-Agent: Mozilla/5.0, and the login POST response status is never checked — auth success is inferred only from the later whoami call. That works, but checking the login response too would fail faster and more clearly...

• extended section // 1. Login to Space-Track, saves cookie, now checks POST failure and login fail

Smaller notes

• noradsToDownload is a module-level variable reused as both an array (group/Space-Track paths) and a scalar (individual path). The mutex serializes states so it's safe today, but it's fragile; the thrown guard in CELESTRAK_INDIVIDUAL_FETCH throws a bare string instead of an Error.

• (noted, not modified)

• convert-csv-to-json is a fairly lightly-maintained dependency for what is essentially CSV-of-known-shape parsing — not blocking, just flagging the supply-chain footprint.

• (noted, not modified)

[accius]

Re-reviewed against 646f015a. Thanks for the thorough pass, Michael — every item from the previous review is addressed, and the fixes hold up on inspection.

Verified resolved:

• ✅ Axios signal now correctly passed inside the config object — client.post(url, body, { signal: controller.signal }) and client.get(url, { ..., signal: controller.signal }). Timeouts will actually fire.
• ✅ 'use strict' added at file top — jsonBytes is now properly declared (const jsonBytes = new Uint8Array(whoami)), and the match line in appendDataToOmmCache no longer leaks since the file would throw under strict mode if it tried.
• ✅ s.data_source?.startsWith('celestrak') — optional-chained.
• ✅ try { ... } catch (ex) { logWarn(...) } finally { return ... } — exceptions are caught and logged before the next state is returned. No more silent swallow.
• ✅ 429 added to the CelesTrak block condition in all three group/individual handlers.
• ✅ appendDataToOmmCache now guards with !Array.isArray(ommJson).
• ✅ Space-Track login response is now checked — non-200 returns the upstream status, and the response body is scanned for failed / denied to catch the case where Space-Track returns 200 with an error message. Good defensive layering with the subsequent whoami check.
• ✅ Comment added explaining ommUnusedCache size is bounded by the group-file size minus tracked sats — fair enough not to add a hard cap.

One concern I'd like fixed before merge — cold-start latency:

The state machine runs purely on setInterval(..., 15 * 1000) with no initial kick (server/routes/satellites.js:571). On a fresh server boot:

1. First sm.run() fires 15s after startup.
2. The pipeline is multi-step (START → AMATEUR_INIT → AMATEUR_FETCH → WEATHER_INIT → WEATHER_FETCH → INDIVIDUAL_INIT → INDIVIDUAL_FETCH → START) — with one transition per 15s tick, ommCache stays empty for at least 60–90 seconds, easily several minutes if any handler short-circuits back to START.
3. Meanwhile the frontend fetches /api/satellites/data once on mount, then every 6 hours (src/hooks/useSatellites.js:38). With no retry on empty payload, a client that hits the server in the first few minutes after a restart sees no satellites — and won't retry for 6 hours.

That's a regression vs the prior endpoint, which blocked on cold-start to guarantee the first request returned data. Two small mitigations would close the gap:

// server/routes/satellites.js
const sm = new StateMachine(smStates, handlers);
sm.run();                                    // kick-start before the interval
setInterval(async () => { sm.run(); }, 15 * 1000);

and / or, in useSatellites.js, retry with a shorter interval while the payload is empty:

const interval = setInterval(
  fetchSatelliteData,
  Object.keys(satelliteData).length === 0 ? 60 * 1000 : 6 * 60 * 60 * 1000,
);

Either alone helps; both together is robust against deploys.

Minor polish (non-blocking):

• logWarn('Error reading CSV:', err) at parseCsvText passes two arguments — most of the loggers in this codebase take a single formatted string, so err is likely dropped. Inline it: logWarn(\[Satellites] CSV parse failed: ${err?.message ?? err}`)`.
• The handler catches log a static "[Satellites] caught unknown exception occurred in <HANDLER_NAME> handler, advancing to next state" — would be more useful with : ${ex?.message ?? ex} interpolated, otherwise operators see "unknown exception" with no detail.
• Space-Track login still uses User-Agent: 'Mozilla/5.0' while every other request (including the Space-Track data fetch) uses OpenHamClock/${APP_VERSION}. If the browser-UA is required by Space-Track's auth endpoint, worth a comment saying so; otherwise consistency would be nicer.

CI is green, the branch is 0 behind / 12 ahead of Staging, and mergeable: CLEAN. With the cold-start kickstart added, this is ready to merge.

— K0CJH

[accius]

I mostly agree with the Automated Triage Review I ran. See notes above.

[MichaelWheeley]

// server/routes/satellites.js

const sm = new StateMachine(smStates, handlers);

sm.run();                                    // kick-start before the interval

setInterval(async () => { sm.run(); }, 15 * 1000);

• add kick start run() after state-machine constructor

in useSatellites.js, retry with a shorter interval while the payload is empty:

const interval = setInterval(

  fetchSatelliteData,

  Object.keys(satelliteData).length === 0 ? 60 * 1000 : 6 * 60 * 60 * 1000,

);

• useSatellites.js fetch data every minute if empty data set

Minor polish (non-blocking):

• logWarn('Error reading CSV:', err) at parseCsvText passes two arguments — most of the loggers in this codebase take a single formatted string, so err is likely dropped. Inline it: logWarn(\[Satellites] CSV parse failed: ${err?.message ?? err}`)`.

• enhanced message on exception in CSV reading error

• The handler catches log a static "[Satellites] caught unknown exception occurred in <HANDLER_NAME> handler, advancing to next state" — would be more useful with : ${ex?.message ?? ex} interpolated, otherwise operators see "unknown exception" with no detail.

• enhanced message on unknown exception

• Space-Track login still uses User-Agent: 'Mozilla/5.0' while every other request (including the Space-Track data fetch) uses OpenHamClock/${APP_VERSION}. If the browser-UA is required by Space-Track's auth endpoint, worth a comment saying so; otherwise consistency would be nicer.

• 'User-Agent': switched from 'Mozilla/5.0' to OpenHamClock/${APP_VERSION}