Mega commit - #22
Merged
Merged
Conversation
… CSVs (fixes Fuel Main flashing open on debug toggle in IDLE)
…im generator) + purple Simulated Data / Session Stopped status badge
…ped WindProfile + /api/drift)
…essure_limits], operator-gated live config editing with download/upload, and dead-config scrub
…try fixes, flight-dynamics graph popups
…esign dialog, trim redundant controls)
…andalone installer and bootstrap step
…e-series overlay + angle-from-vertical, square ground track, larger legible charts
…ments, identifiers, deploy configs, openrocket subdomain + auth slug, landing card)
…der backend/recovery, tools/site-climatology/reference, unified health, tests (5 frontend-coupled quarantined for Phase 3)
…tabs, unified OrkConfig design + single header design bar, recovery Inter/engineering-palette index.css, UnitsProvider, vitest
…sign (Flight Dynamics apogee + CAD structure/spent-motor mass) or typed in; lateral velocity deferred
…/Caddy subdomain, fold userdata volume + OPENROCKET_S3 into star-openrocket-api, drop landing card, delete source
…g cad/recovery slices, never throw); remove dead recovery storage.py (unused RECOVERY_S3); add legacy-blob persist test
…e (fixed-height status line) so editing a value no longer jumps the page
…nt no longer forces a triple-click)
… verbose-gated), unify verbose flag, demote per-packet/hex/debug noise to verbose, throttle OTA progress to 25%
…espace Diablo:: -> daq::)
…d by config mode byte (0-3), 1Hz buffered flush + ethernet-up announce
…only (/api/onshape, /api/build); any Berkeley login can use recovery/flight/cached models; friendly 403 in the model picker; landing copy
…strip eq/section refs from UI text
…grate the horizontal ground track (loads use resultant deployment airspeed, drift reads the track, shear lag modeled), lateral velocity at apogee on Vehicle; crosscheck drives ours+mastersheet from config wind; calm = byte-identical
…+crosscheck see the same wind), lateral velocity at apogee input + from-design toggle (RocketPy apogee velocity), design source wiring
…backend cache→GUI viewer + per-board log-mode control (0-3) + integration coverage; config editor read-only/cleanup
… the Vite migration, smol-toml and the config-driven [[states]] rework, so six of its sixteen items were already fixed; those are recorded with how rather than dropped, and the new audit adds the sequencer concurrency races, the heartbeat's hardcoded state-enum mapping, read_packet's stream desync, the unbounded WS broadcast and the static-analysis job that cannot fail
… broadcast queueing behind housekeeping — a thread per TCP client plus FireManager's expiry thread ran transitionTo concurrently and double-joined the actuator republish thread (confirmed under TSan: pre-fix dies in 0.2s at ~50 commands, a plain -O2 build survives 1016 because glibc tolerates it, which is worse); a mutex deadlocks here since fire_manager_.stop() joins the very thread waiting on it, so commands now go through a queue the timer posts to without waiting, ABORT goes out first instead of behind an unbounded controller connect (measured 133,348ms), and reload re-resolves [fire] and stops the republish loop before rewriting the tables it reads
…ntegration fire gate passing vacuously — both new tests were validated by building them against the pre-fix sources (concurrency dies at the same stopContinuousLoop double-join, abort ordering measures 453ms vs 0ms), the TSan job gates on exit code rather than grepping for "WARNING" because the failure is a sanitizer CHECK abort and needs setarch -R or it never starts, test_robust_ddp is finally registered with CTest after being built-but-never-run, and the harness now rewrites [controller_service].port so FIRE_START/FIRE_STOP actually reach the controller instead of being sent to 9999 while it listens on 9997
… ten items move to a dated Resolved section recording how each was fixed and what the measurements were, the ABORT and sanitizer entries narrow to the halves still open (broadcast routing; -Wall and ASan/UBSan, since only TSan landed), test_robust_ddp drops out now that it is registered, and the four line anchors my own edits shifted are re-verified
…reconnect — config now reaches a running rig at exactly one point, the deploy at session start, and the pipeline services hold what they booted with; the reload verb could never reach a live sequencer anyway (its only caller fired when the CSV was deployed, which only happens with no session active) so it was untested machinery in the safety-critical path, and deleting it also removes the reload half of 0c0dfd6; tryConnectElodin() now reuses an actuator-board snapshot from init() instead of re-parsing config.toml on every reconnect, which had the retry thread rebuilding the actuator tables from whatever was on disk when a db blip happened mid-run
…eploy fails, and stop board log mode reverting itself — the always-on backend cannot read config once at boot the way the session-gated services do, so readDeployedConfig() caches and deployActiveProfile() invalidates, which removes a file read + TOML parse from every /api/gui-config request, every operator command and all 24 resubscribe retries; a failed deploy at session start now aborts the start instead of warning and running the rig on stale config behind a green "session active" light; and patchBoardField() writes the active profile as well as config.toml, since board log mode is a deliberate live exception but was being silently reverted by the very next deploy
…, and warn that sim ignores it — the API already returned a deployed flag and a message ("Saved as draft (applies at next session start)") and the banner threw both away for a fixed "saved successfully", so an operator saving mid-session was told the change was in effect when it was not; the banner now uses the server's own wording in amber, a new one lists the profile files that differ from what is deployed via GET /api/config/profiles, and the session page notes that a simulated run uses the committed sim config rather than the profile, which otherwise reads as the draft mechanism being broken
…, deploy, session-start and name the two exceptions — the old guide described a flight/ground split config that has not matched the profile system for months, so it documented a shape of the system that no longer exists; the rewrite says which of the three files called "config" is which, that session start is the one apply point, and that board-config broadcast and calibration are deliberate live exceptions (including that [abort_pts] and boards.*.enabled reach hardware live), plus the two traps worth knowing: the freeze is inert wherever SESSION_SERVICE_MODE is off, and sim never applies drafts; SENSOR_ASSIGNMENT_SYSTEM.md gets an aspirational banner because its MAC-hash assignment flow is not what ships
…oped wrong: no socket in the DAQ path bound an egress interface, which only stopped mattering because the DAQ owned its machine and started mattering again on the shared apps box (board LAN + site LAN + docker bridge); one resolver picks the interface holding the board subnet by address the way bootstrap_daq.sh already did, refusing to start on an explicit bind_ip that is not on the host or on two candidate NICs, and binding a source address rather than SO_BINDTODEVICE because the units run unprivileged with no CAP_NET_RAW; the abort now takes its destination and port from [server_heartbeat] instead of 255.255.255.255:5005, which needed no config change since every profile already sets it; and the firmware turns out not to act on ABORT at all (ENABLE_ALL_STATE_TRANSITIONS is false everywhere, which also makes the [abort_pts] trips unreachable) while CLEAR_ABORT is consumed and never sent, so the entry is downgraded and that is filed instead
…in the same calibration page
…tate was 0 forever — it consumes [0x50,0x00] and called subscribe_stream(), whose doc comment promised "all stream data" but which actually sent the calibration service's list (480 raw sensor ids from a hardcoded boards 1-8 x channels 1-10 guess, plus the calibration command table); measured with the real binaries against a real elodin-db, four transitions produced 101 heartbeats all carrying 0, at 0% CPU with the Elodin thread parked in recv() and nothing logged, and it now reports {0,1}; subscribe_stream() is deleted and every consumer names what it reads via raw_sensor_tables()/calibrated_sensor_tables() built from the boards actually in config, the way ControllerService already did; also fixes the read path this was originally filed against, which was real but never firing — an oversized packet abandoned its payload in the socket and desynced the stream permanently, the bound was off by four so packet_len == max_len overran the caller's buffer (EFAULT against a guard page), read_exact returned TIMEOUT after a partial read for the same desync, and only the write path ever cleared connected_ so a dead socket read as live; the 256-byte buffer this file blamed was never overflowed, nothing emits over 179 bytes, and that claim is withdrawn
…handoff had never once engaged — an [actuator_roles] entry now takes an optional 4th element ("pwm_fuel"/"pwm_ox") assigning that actuator to controller_service, replacing five separate fallbacks in controller_main (a missing role became CH3/CH8 on board 12, an undeclared board became "192.168.2.<board_id>", an empty board table became a synthesized 11-14, two roles on two boards silently shared the first one's IP, and PWMConfig's own rig-shaped defaults gave each of them something to land on), every one of which drove PWM onto whatever hardware happened to sit on the guessed channel behind a warning nobody reads; the same field is what ActuatorCommander reads for is_pwm, which is the real fix for the sequencer/controller handoff — it keyed on kind == "PWM", and no shipped config could set that without losing the NC/NO polarity that shares the slot, so "LOX Press" was normally-open and therefore never marked PWM and the handoff (correct code, with a passing test) had never engaged on a real rig; unresolvable config now disables the fire gate via setFireStateId(255) rather than guessing, resolution moves to PWMTargets.hpp behind test_controller_pwm_roles (9 cases / 26 assertions, mostly pinning the absence of fallbacks), and sendActuationPWM splits the datagram when the two targets sit on different boards; also in the controller: the control server sets SO_RCVTIMEO on the accepted socket and handles each client on a capped thread pool, so a client that connects and never sends a newline can no longer park the accept loop, notifyControllerFire reads the OK ack it always threw away (retrying once, loudly) so a wedged controller is no longer invisible at both ends, and the Elodin subscriber — the only consumer that started once and let its thread die on the first db blip — now reconnects and re-subscribes; the editor assigns on the Actuators tab and blocks the save, the API rejects the same on POST and import, and digital-twin deliberately assigns neither because it declares no press valves
…, duplicate state ids, a fire timer expiring into a refused transition and an unassigned PWM output in red next to the field they concerned, and an operator could read that, navigate away, and start a run on exactly that config; three of the rules blocked the editor's own Save button and none of them blocked session start, which is the one point where the active profile is copied into config.toml and the C++ services read it, so config arriving by import, CSV upload or a hand-edited profile file never passed a guard at all; the rules move to shared/config-validation.ts as pure functions (config + the three state CSVs in, ConfigIssue[] out) and SessionManager.start() refuses on them — the enforcement point and the only one, since a browser cannot skip it by not asking — answering SESSION_START_BLOCKED with the list, which the session page renders grouped by the config page that fixes each item with a link to /config?tab=<id>, and a second press sends force and runs anyway; the gate runs before any field of SessionManager is assigned, so a refusal leaves active/dbDir/deadlineMs untouched and config.toml unchanged, which the pre-existing failed-deploy path did not — it mutated them first and left a dbDir for a run that never began, and is now behind the same guard; two deliberate calls: warnings block as well as errors on the first press only, because a gate firing on errors alone would leave "no state is flagged Abort" exactly as advisory as before, and simulated runs are not gated for the same reason they skip the deploy, since they read the committed config_base → sim_config overlay and never touch the profile, so gating them would block on config that is not in effect and teach operators to press Start twice by reflex; the editor drops its local parseCsvGrid/serializeCsvGrid/diffKeys/boardSlotIssue/boardDisplayName for the shared ones and builds its tab bar from CONFIG_PAGE_LABELS, so an issue's page is by construction a tab that exists; covered by 19 vitest cases on the rules (about half of them pinning the quiet cases, since a false positive is what trains an operator to ignore the gate) and 6 hermetic backend cases on the gate itself, 4 of which fail against the pre-gate code, plus GET /api/config/validate asserted in the integration run — which immediately reported that the harness's own [boards.integration_startup] board_id = 60 maps to Elodin slot 10, outside the 1-8 a packet id can encode
…tarting on any box that actually has the DAQ NIC — the harness talks to a simulator on loopback but left [network].bind_ip at 0.0.0.0, which sends the services down the auto-resolve path added in 810cfd4, and with eth2 192.168.2.20 present alongside lo the configured board subnet 192.168.2.0/24 is reachable on two interfaces, so sequencer_service and daq_bridge both refused to start rather than guess an egress interface; the refusal is correct and is the whole point of that change, so this pins bind_ip = "127.0.0.1" in the generated test config for the same reason the harness already pins broadcast_ip to localhost — the test knows it runs on loopback, it just never said so
…redAmount, so a client that stopped draining had every subsequent frame queued in the backend's heap without limit — ~47 live streams at 20 pts/s is ~124 KB/s per stalled client, ~460 MB/hour, and this is the one process feeding every dashboard, which is why the standing workaround was to restart it; the memory was the lesser half, because a FIFO queue guarantees completeness and therefore gives up currency without bound: the tablet renders a smooth, plausible, coherent plot that is minutes old with no gap, no stale badge and readyState still OPEN, which on a test stand is worse than a blank screen, and against a 50 KB/s link it never recovers (lag 60 s at t=60, 96 s at t=120, 203 s at t=300) because drain rate is permanently below production rate; sensor samples now stage in a per-client outbox as min/max windows in a tiered ladder where an overflowing level merges its two OLDEST windows into one promoted a level up, so a window only ever merges with a same-resolution neighbour and old data cannot collapse — two simpler schemes were simulated and rejected for representing a 36-second burn with a single window — and merging keeps each extreme with its ORIGINAL timestamp, so a 620 psi ignition spike survives any number of compactions at its true time, which is why this is min/max decimation and not last-value conflation; before each flush the ladder is squeezed to a latency budget taken from the client's measured drain rate (bytes handed to the socket over the time the socket took to empty), so what does not fit is compacted away rather than delayed, inverting the control law from fixed buffer/variable lag to fixed lag/variable resolution — at a 1.5 s budget lag holds at <=1.6 s across 2000 to 20 KB/s while resolution degrades 100% to 16%, and a healthy client is untouched because its outbox holds one window and the compaction path never executes; two things had to be right for it to be safe: the flush fires only when the socket has actually DRAINED rather than merely dropped below a low-water mark (simulated with a 32 KB mark the socket queue climbed 42 to 93 to 152 KB, reintroducing the same unbounded queue one layer down), and only SENSOR_UPDATE passes through the outbox while NOTIFICATION/STATE_UPDATE/SESSION_UPDATE/ACTUATOR_UPDATE keep going out directly, because sensor samples are idempotent and the next supersedes the last but a state transition or an abort notification is not; also the 30 s ping/pong reaper so a peer that vanished without a FIN stops living in wss.clients forever, SENSOR_DATA_STALE_MS moved onto the server timeline so a throttled client reading current-but-coarse data does not blink "Data Pipeline Down" between batches, and a Throttled - N% badge extracted into lib/connection-badge.ts from the nested ternary duplicated verbatim in TopBar and MobileDashboard, ranked below Disconnected/Data Pipeline Down so it cannot mask an outage and orange because yellow already means the pipeline is down, with every value on it computed in the backend since resolutionPct needs a produced-count the browser never receives and lagMs needs the server clock; encoders were exempt from every cap and are now capped by their own [gui] encoder_points_per_second (default 100) rather than the 20 pts/s GUI budget, because OscopeTriggerPlot measures valve actuation timing from those samples and 100 ms windows would quantize it away — at the boards' ~48 Hz it is one sample per window, min == max, a ceiling against a faster board and not a downsample of this one; SUBSCRIBE_SENSOR/UNSUBSCRIBE_SENSOR are deleted rather than implemented, having had two senders and zero consumers and filtering nothing even if honored since the client asked for every channel at connect and never revised it; /api/debug reports wsBufferedBytes, outboxWindowsHeld and heapUsedMb so "the dashboards are stuck" is one curl instead of a restart, asserted in the integration run, and covered by 19 backend cases on the ladder and the squeeze plus 11 frontend cases on the badge precedence, of which 5 fail with the latency budget disabled, 1 with the drain reversed and 2 with the extreme timestamps dropped
…n as `--error-exitcode=1 ... || true` so it could not fail, and measured against the same cppcheck 2.13.0 ubuntu-latest ships it produced 199 messages with zero `error` and zero `warning` (146 `missingInclude` noise from having no `-I`, leaving 47 style / 3 performance / 1 portability across 28 files), so gating meant carrying suppressions for `missingInclude`, `useStlAlgorithm` and `constParameter` — the last being nine `char* argv[]` on `main` — and triaging ~20 leftovers whose two most bug-shaped members are false positives (`CubicCalibrationStore.cpp:57` is a deliberate `amin = amax = pts[0].adc` init, and `daq_bridge_main.cpp:345`'s always-false condition holds only because cppcheck cannot see through `load_board_map_from_config` without include paths); clang-tidy was worse than ungated — the step passed `-p build` but the job never built anything and nothing in the repo emits a compilation database at all, `CMAKE_EXPORT_COMPILE_COMMANDS` being set in neither CMakeLists.txt nor the workflow while the build job uploads only bin/ and lib/, so `-p` named a directory that did not exist, clang-tidy fell back to the bare `-std=c++20` and every file died on its first `#include`, meaning the `|| true` and `continue-on-error: true` were hiding a step that had never produced a finding rather than one whose findings were ignored; build-summary drops the dependency and its row, the three cppcheck findings worth keeping are refiled under Low (ElodinClient.cpp:197-199 decodes packet_type/packet_id/request_id and reads none, ControllerLUT.cpp:133 tests an unsigned for < 0, calibration_main.cpp:1336 casts const unsigned char* to const float*), and the IMPROVEMENTS entry is retired to Resolved pointing at the ASan/UBSan work as the better place to spend the effort
…traffic rather than at ctest — CMakeLists set no warning flags at all, so the cheapest analyser available was running silent; -Wall -Wextra -Wpedantic -Wno-unused-parameter with -Werror=return-type as the only error (a value-returning function that falls off the end leaves the caller reading a register and has no benign form) reported 40 warnings on a still-green build, and they were not noise: ElodinClient.cpp:197-199 decoded packet_type/packet_id/request_id and read none of them, daq_bridge_main.cpp:581 declared last_config_save and never read it while FSWConfigManager exposes no save API at all so it was leftover rather than a missing feature, calibration_main.cpp carried a dead lp_pt_psi_before_offset obsolete since Zero All was reworked to capture a 0 reference through the shared fit instead of subtracting a tare, and all 22 -Wmissing-field-initializers in SensorAssignment.cpp traced to PressureSensorSpec::calibration_file and requires_calibration, which nothing in the tree reads, so they are deleted rather than suppressed; the two suppressions that remain are scoped and deliberate — the archived Elodin headers become SYSTEM includes since db.hpp was the only other source of that warning, and daqv2_comms carries -Wno-stringop-overflow for the GCC 13 false positive on the vector::resize calls at DiabloPacketUtils.cpp:748,769 that are guarded by `if (N)` and a MAX_ bound — and the tree now builds at 0 warnings; -DSANITIZE=ON adds ASan+UBSan and forces CMAKE_POSITION_INDEPENDENT_CODE, which is required not optional because daqv2_comms is a static library linked into a shared one and ASan's __asan_option_detect_stack_use_after_return reference fails to relocate without PIC, a hard link error; the new address-sanitizer job runs test_integration.sh rather than ctest and that choice was measured, not assumed — all 10 ctest tests pass under the sanitizers with zero findings, while one integration run, which drives real UDP through every service, reported what nothing else had: BoardDiscovery::process_board_announcement declares `DiscoveredBoard board;`, assigns six of its nine fields and hands the whole struct to add_or_update_board, which stores it, so supports_dynamic_config, max_sensors and active_sensors were copied uninitialised — UBSan caught it as "load of value 212, which is not a valid value for type 'bool'", but the bool is the harmless one since nothing reads it, whereas max_sensors and active_sensors ARE read at BoardDiscovery.cpp:380-381 and DynamicConfigManager.cpp:107-108 where they are serialised into the generated board config — though both routes to that serialisation, BoardDiscovery::generate_config and DynamicConfigManager::update_from_discovery, turn out to have no callers at all, so nothing ships the garbage today and this is a landmine rather than a live fault; the uninitialised read itself is live on every board announcement, and the moment either config path is wired up it would ship a stack byte as a board's sensor count; fixed with default member initialisers on BoardSignature, SensorInfo and DiscoveredBoard so a future construction site cannot reintroduce it, and the misaligned u32 stores in config_broadcast turn out to be five sites rather than the three on file (line numbers had drifted to :357,364,373,376,395) plus the same class at calibration_main.cpp:1336 and an unaligned LOAD at ElodinClient.cpp:196 that was never recorded, all now memcpy, which compiles to the same instruction with none of the undefined behaviour; also ControllerLUT.cpp:133's `hi <= 0` on a size_t is written `hi == 0`; the job comment spells out three traps because each costs an afternoon — setarch -R as in the TSan job, SANITIZE=ON needing PIC, and above all that UBSan's non-fatal diagnostics never reach the job's stdout but go to each service's own stderr and thence to .tmp/integration_*.log, which log_path does not capture either, so the script exiting 0 proves nothing and the grep over those logs is the actual gate — this was nearly filed as "zero findings" off exactly that mistake; INTEGRATION_SENSOR_HZ is pinned to 5 and boxed in from both sides, since at full rate under ASan a local run measured 96.8% frontend broadcast delivery against the harness's >=97% assertion while at 3 Hz the harness fails the opposite check with median stream spacing 1005 ms against its 5-1000 ms window; verified at 0 warnings, ctest 10/10 under sanitizers, and test_integration.sh green both sanitized (zero reports across every service log) and unsanitized (246 checks), which empties the High section of IMPROVEMENTS.md
…d not run — clang-format's peak memory is set by the worst file in the tree and format.sh was feeding it two vendored single-header libraries that its prune list never covered, diablo_server/lib/include/nlohmann/json.hpp at 899 KB and toml++/toml.hpp at 475 KB, both of which blow past 1.3 GB and one of which was measured at 5.6 GB RSS on its own — enough to invoke the OOM killer on a 7 GB dev box, which took systemd with it and tore down the WSL VM; reformatting them was always wrong anyway since they are upstream code, so nlohmann, toml++ and node_modules join the prune list (node_modules for both the C++ and Python walks, since neither excluded it and diablo_server has two of them), and FORMAT_JOBS stops defaulting to bare nproc — 22 clang-format processes against 7 GB is a reliable way to reach the same place — capping at 8 unless the caller knows better; with the bombs pruned the check runs to completion over 241 files and reports the five files that were actually unformatted, all from 2dfccb9 and none of them new, so they are formatted here and the Code Formatting Check job goes green for the first time in at least four runs; separately, ThreadSanitizer (sequencer) has never executed a single test — its run step said `working-directory: build-tsan`, but a step-level working-directory REPLACES defaults.run.working-directory rather than nesting under it, so it resolved to $GITHUB_WORKSPACE/build-tsan while the build step, which used the default, had created daq-server/build-tsan, and the job died with "No such file or directory" before running anything, meaning the sequencer double-join regression this job exists to guard has been unguarded since it was added; the same class of bug as the clang-tidy step deleted in 741c466, and both concurrency tests pass locally under TSan with zero warnings once the path is right; also the asan-service-logs artifact uploaded nothing on its first run because .tmp is a dotted directory and upload-artifact v4 excludes hidden paths unless told otherwise, which matters precisely when the job fails and someone wants the logs — the gate itself was fine and correctly reported "no sanitizer reports across 11 service log(s)", so that job was genuinely green rather than falsely green
swissskimmilk
force-pushed
the
star-openrocket-merge
branch
from
September 11, 2026 05:03
521113e to
3adfe6d
Compare
…ate mechanisms were taking a design out from under someone mid-edit, and none of them said so; the loudest is that `visibilitychange` called releaseCheckoutOnUnload, which is an explicit handback to the server rather than a local state change, so alt-tabbing to a datasheet for three seconds, locking the screen or closing a lid gave the design away instantly and the user found out on return — that one is already fixed on this branch (pagehide only) along with lock_ttl 300 -> 900, but two more remained: `_beat_lock` was reachable only from /autosave, so "inactivity" meant "no change to the saved payload" and panning, measuring, reading a result or thinking all counted as idle, and useCheckout stopped polling the moment it held the token on the reasoning that its own saves keep it — so when a hold did lapse the canvas stayed editable and the only signal was a save returning 423, after the user had typed into a void; a content-free POST /checkout/beat now refreshes the hold without touching the payload (423 when you do not hold it, which is the client's cue to stop rather than beat into nothing), the hook beats on real interaction while the tab is focused and re-checks otherwise on one 15 s timer, and activity within idleCapMs (15 min) holds it indefinitely while genuine idling still frees it, because release is holder-only and the on-close beacon is best-effort so the TTL remains the only thing that recovers a checkout after a crash; lockExpiresAt was also simply wrong — it carried the raw lockHeartbeat, a timestamp already in the past, so anything counting down to it read as expired the instant it rendered, which is why no countdown had ever been built — it is now beat + lock_ttl with lockTtlSeconds alongside, and the bar shows "Editing · 4:31", turns amber under two minutes and offers Keep editing; and because a lapse has to survive the user being elsewhere, CheckoutLostDialog raises a persistent in-page dialog with Take it back, flips document.title to "⚠ Checkout lost — …" so it is visible from another tab, and posts a desktop notification where permission allows — permission asked on the Take click, which is a real user gesture, never on load — with the notification explicitly best-effort since a closed laptop shows nothing either way and the dialog is the mechanism; a deliberate Release is not a loss and raises none of it; covered by four new cases across all three apps' checkout suites, of which test_lock_expires_at_is_in_the_future fails against the previous semantics, plus 28 passing server cases, 156 frontend cases and a clean tsc
…ting — the hook was the only place the new logic lived and there is no jsdom, no testing-library and no component or hook test anywhere in this repo, every existing suite being a pure-function test in a lib/ directory, so rather than add a DOM stack for four arithmetic decisions the decisions move to lib/stardesign-ui/src/checkoutPolicy.ts (secondsLeft, isExpiringSoon, shouldBeat, isUnexpectedLoss, mmss) and useCheckout/CheckoutControl consume them, which also stops the warn threshold and the mm:ss formatter existing twice; 17 vitest cases pin them and were mutation-checked rather than assumed — dropping the "we held it" half of isUnexpectedLoss, flipping shouldBeat's boundary from < to <=, removing the unparseable-date guard in secondsLeft and removing its clamp at zero each fail exactly one case, and the suite is green with all four restored; on the server the beat endpoint gains four cases across all three apps' suites and removing the route fails exactly those four, including the end-to-end claim this work exists for: beating twice without ever saving holds the design past its TTL, and stopping frees it — both directions, since lapsing is the only thing that recovers a checkout after a crash given release is holder-only and the close beacon is best-effort; two things surfaced only by running the other apps' suites rather than assuming one copy stands for three — pid-designer's payload is {nodes, edges} not {config}, so the "a beat does not clobber the working copy" assertion now compares the stored body against itself and is shape-agnostic, and a hold that has ALREADY lapsed cannot be beaten back to life (423), which is correct and now pinned, because a client that could beat its way back would reintroduce the two-holders case checkouts exist to prevent, and it is precisely why the lost dialog offers Take it back rather than silently re-beating; 30 server cases per app across all three, 173 frontend cases, tsc clean, format clean
…oth of them mine, both from verifying one app and assuming it stood for three; the route-inventory guard (test_every_doc_scoped_route_is_listed) pins the doc-scoped matrix against the real route table precisely so that adding an endpoint fails until someone says which side of the access boundary it sits on, and /checkout/beat never got classified, which failed star-openrocket, EngineDesign and pid-designer alike — the last one only via tests/test_diagrams.py rather than tests/test_documents.py, which is why a filename-shaped first pass missed it; beat_checkout is doc-scoped (it 403s an unshared caller and 404s a foreign id without owner, both of which the matrix now exercises for it); separately the EngineDesign Playwright suite drives its checkout by polling getByText('Editing', { exact: true }), and appending the countdown as a bare sibling made the chip read "Editing · 14:59" so the exact-text locator stopped matching and three specs failed on a 30 s timeout each — a slow and very indirect way to learn that a label changed — so "Editing" becomes its own element again and the countdown stays a separate span, which keeps their assertion honest rather than loosening it to fit me; that contract now has a fast guard of its own: renderToStaticMarkup needs no DOM, so five vitest cases in star-openrocket pin the chip markup the E2E depends on, and reverting the span to a bare text node fails two of them immediately instead of costing a Playwright run; the whole sweep this time rather than one file — 628 + 100 python cases (EngineDesign's 6 collection errors are pandas/python-multipart missing from the venv being borrowed, not this change; its own two suites pass), 178 frontend cases, tsc clean, format clean
…ck to the tab, and it could not be trusted across two clocks — three defects, all mine, all in the wiring rather than the logic; the visible one: lastActivityRef was seeded with Date.now() at mount, so merely opening the page counted as interaction and every 15 s tick beat the lock for a further idle window, which is why leaving a tab alone and returning showed a fresh 15:00 with nothing touched — it now starts at 0, because pressing Take is the user doing something and loading a page is not; second, pointermove was in the activity set, so incidental cursor travel re-armed another full window, and since the idle cap is the ONLY thing that frees a design from someone who walked away (release is holder-only, the on-close beacon best-effort) a parked tab held it against everyone else indefinitely — the set is now pointerdown/keydown/wheel, which still catches every real action (typing, clicking a tab, starting a CAD orbit, zooming, beginning any drag) while ignoring a wandering mouse; third, a hidden tab still runs its timers, so shouldBeat now declines while document.visibilityState is not visible — declining to REFRESH, never to release, which is the distinction that made a three-second glance at another tab cost someone their design in the first place; separately the countdown differenced a server timestamp against the browser's own clock, which is only correct if the two agree and nothing makes them: a browser ~17 h behind its WSL backend displayed "1038:44" left on a 15 minute hold, and one running fast would have sat at 0:00 with the warning pinned on, looking exactly like the lapsing bug this work exists to fix — _lock_state now also returns lockExpiresInSeconds as the server measured it and the client subtracts only locally-measured elapsed time, so skew cancels in both directions, with the old timestamp kept as a fallback; 27 policy cases now, of which reverting the duration path fails 4 and dropping the visibility guard fails 1, plus 188 frontend and 628+100 python cases, tsc and format clean — but the seeding bug itself is NOT covered, because it is hook initialisation rather than pure logic and this repo has no jsdom or react testing stack anywhere, which is the second bug in a row to land in exactly that blind spot
… checkout bug that reached a user lived in the one layer nothing could see — three defects shipped from this work and not one was a logic error: the chip's text changed and broke an exact-text locator, `lastActivityRef` was seeded with Date.now() at mount so opening a page counted as working and the countdown reset to a full 15:00 forever, and the countdown differenced a server timestamp against the browser clock and read "1038:44" when the two disagreed by 17 hours; each is React wiring (a ref's initial value, a listener set, a timer, an effect) and the 188 existing frontend tests are all pure functions under vitest's default node environment, so none of them could reach it — two were found by a person watching a timer and one by a 30 minute Playwright run after push; worse, a test placed inside lib/stardesign-ui/src was executed by NOBODY, since every app's vitest include is rooted at that app's own frontend directory, so the library feeding three apps had nowhere to test itself; it becomes a package: package.json + vitest.config.ts (jsdom, mirroring daq-server/diablo_server/frontend which is the working precedent for this stack here) + a tsconfig.json so `npm run lint` is a real gate rather than the no-op it was without one (verified by planting a type error), with the suites under test/ rather than src/ — not cosmetic, since all three apps include ../../lib/stardesign-ui/src in their tsconfig and type-checked the jest-dom matchers they have no types for, which broke `tsc -b` in all three until the move; 68 cases now, covering what the pure decisions could not: an untouched tab never beats, a drifting pointermove is not work but pointerdown/keydown/wheel are, a hidden tab stops refreshing yet never RELEASES (the distinction whose absence made a three-second glance at another tab cost someone their design), a 17 h skew still reads 900 s, a 423 or a poll reporting someone else's name raises the lost dialog while a deliberate release does not, and the dialog's title mutation, its restore on unmount, its buttons and its best-effort notification; four mutations were checked rather than assumed — reinstating the seeding bug fails 2, restoring pointermove fails 1, dropping the visibility guard fails 2, releasing on visibilitychange fails 1; one Playwright spec earns its place because fake timers are exactly what would hide a real-timer defect, and getting it to bite took three attempts worth recording: fast-forwarding past the idle cap first retires the bad mount stamp so the test passed either way, and a reload cannot reproduce the case at all because pagehide releases on the way out — it takes a second tab, which mounts already holding the design having been touched zero times, and that version does fail against the bug; its countdown assertion is deliberately weak because the owner tab is legitimately refreshing the hold, so the request count is the exact statement and the arithmetic stays pinned in the unit suite; CI gets a stardesign-ui workflow gated to lib/stardesign-ui/** alone, and pid-designer and engine-design — which watched lib/stardesign/** but not lib/stardesign-ui/** — now trigger on it too, closing a hole where a shared-UI-only commit skipped EngineDesign's Playwright suite entirely, the one thing that caught the chip regression and only because that commit happened to touch EngineDesign/** as well; verified across all three apps this time: lib 68 + tsc, tsc -b clean in each, 156/10 app vitest, 628/100/355 python, 6/6 EngineDesign E2E twice over, format clean
… a user sitting with the tab open and touching nothing watched the countdown bounce 15:00 -> 14:45 -> 15:00 indefinitely, and the cause was my rule rather than any leftover bug: shouldBeat kept the hold alive while the last interaction was younger than idleCapMs, and idleCapMs was 15 minutes, the lock's own TTL, so a single press of Take licensed a full TTL of automatic refreshing and the countdown could never visibly fall; the same user noticed it ticked down correctly after minimising the tab and returning, which is the diagnosis confirming itself — the visibility guard suppressed the refreshes while hidden and the countdown was finally free to run, while a visible tab had them written back over the top every 15 s; the rule becomes "has anything happened SINCE the last refresh", tracked with a lastBeatAtRef alongside lastActivityRef, which needs no window and no tuning: do something and the next tick refreshes, stop and the countdown runs down honestly to zero and the design frees itself, which is also what was asked for — editing, switching a GUI tab, rotating the CAD model or any real action resets it, and nothing else does; idleCapMs is deleted rather than retuned, since any value for it is either too short (losing a design mid-task) or too long (this bug), and no consumer passed one; keepAlive stamps the beat too, so the Keep editing button cannot be double-counted, and it now routes through applyState so the countdown re-anchors to the server's fresh duration rather than only the old timestamp; covered by 68 cases of which 4 fail against the window rule, including one asserting exactly the reported symptom (Take, then ten idle minutes, exactly one refresh) and one that a new keydown immediately earns another; 6/6 EngineDesign E2E, tsc -b clean in all three apps, 156/10 app vitest, format clean
Carlsaurus
added a commit
that referenced
this pull request
Sep 12, 2026
Upstream's "Mega commit (#22)" reworked the design checkout (beat on activity, lost-hold dialog, pagehide-only release, a vitest suite for lib/stardesign-ui) and folded recovery-calculator and onshape-viewer into star-openrocket. Resolutions: - lib/stardesign-ui/useCheckout.ts: upstream's hook, with the branch's local-machine behaviour re-added as an opt-in `local` option (default from the hostname): take on open, beat every tick, take back on lapse. The lib's vitest config pins a deployed jsdom URL so the suite tests the deployed model; local mode has its own cases. - docker-compose.yml, Caddyfile, setup.sh, README.md: upstream's star-openrocket entries plus the branch's feed-twin ones, images tagged ${STAR_IMAGE_TAG} like the rest. - format.sh: upstream's node_modules/vendored prunes plus the branch's .venv/__pycache__/egg-info prunes for the Python targets. - PRNode.tsx: the branch's lettering-outside-rotation symbol keeps upstream's plain top port (id "t") when the regulator has no dome.
Carlsaurus
added a commit
that referenced
this pull request
Sep 12, 2026
Upstream's "Mega commit (#22)" reworked the design checkout (beat on activity, lost-hold dialog, pagehide-only release, a vitest suite for lib/stardesign-ui) and folded recovery-calculator and onshape-viewer into star-openrocket. Resolutions: - lib/stardesign-ui/useCheckout.ts: upstream's hook, with the branch's local-machine behaviour re-added as an opt-in `local` option (default from the hostname): take on open, beat every tick, take back on lapse. The lib's vitest config pins a deployed jsdom URL so the suite tests the deployed model; local mode has its own cases. - docker-compose.yml, Caddyfile, setup.sh, README.md: upstream's star-openrocket entries plus the branch's feed-twin ones, images tagged ${STAR_IMAGE_TAG} like the rest. - format.sh: upstream's node_modules/vendored prunes plus the branch's .venv/__pycache__/egg-info prunes for the Python targets. - PRNode.tsx: the branch's lettering-outside-rotation symbol keeps upstream's plain top port (id "t") when the regulator has no dome.
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
1 participant
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.
Another mega commit. Change includes
STAR ORK
DAQ Server
CI and Tooling
Ops and Deployment
DAQ Run viewer
Firmware