Tab-completes the script's named options and, intelligently, their
values against the git repo containing the CWD:
- remote-name options complete against 'git remote';
- remote-branch options (--upstream/downstream/fork-branch) complete
against the branches OF THE RESOLVED REMOTE, resolving each role to
a remote with the same precedence the script uses (explicit
--*-remote on the line > MERGE_U2D_* env var > built-in default);
- --local-*-branch options complete against local branches.
Install by sourcing from ~/.bashrc or symlinking into a
bash-completion completions dir under the command's basename.
Assisted-By: Claude Opus 4.8
Add MERGE_U2D_* environment variables that seed the default for every
command-line option, with precedence CLI option > env var > built-in
default. This lets the script run with no arguments when e.g.
MERGE_U2D_FORK_REMOTE is exported from .bashrc; -h shows the live
resolved defaults.
Also fix the step-7 fallback so that when 'gh pr create' refuses because
an open PR already exists for the fork head (the older-head path, where
the push already updated it in place), the existing PR is found and its
URL reported instead of erroring. 'gh pr list --head' filters by branch
name only, so match on the bare fork branch plus a headRepositoryOwner
filter, mirroring the early idempotency check.
Assisted-By: Claude Opus 4.8
A regression: replacing 'if: always()' with the bare content guard
'if: ${{ !(needs.build.result == '\''success'\'' && ... skip_tests == '\''true'\'') }}'
lost always() semantics. Because 'build' is in needs:, a FAILED build then
cascade-skips build-gate (GitHub's default: a dependent job is skipped when a
needs: dependency fails unless the if: contains always()/!cancelled()). That
left the REQUIRED build-gate check quietly pending on a real build failure
instead of going red (observed on run 33162860634, a push to main with a
failing build).
Restore always() so the gate runs for every build outcome and the narrow guard
alone decides the sole non-run case (a *successful* compile-only build, which
must stay pending/blocked, not red). A real build failure now runs the gate and
it exits 1 -> red. Documented why always() is required in the comment.
Assisted-By: Claude Opus 4.8
A successful compile-only run (fork pull_request, *-reviewed-for-compile tag,
or workflow_dispatch with skip_tests=true) previously made build-gate exit 1:
the required check went red AND the whole Actions run showed as failed, even
though compilation succeeded. That reads as breakage when nothing broke.
Fork pull_request runs get a read-only GITHUB_TOKEN (permissions: is capped to
read-only regardless of what the workflow requests), so the Commit Statuses and
Checks APIs both 403 there — a posted 'neutral'/status cannot mark the check.
And neutral would not block merge anyway (GitHub treats neutral/skipped/success
as passing for required checks). The only state that is both non-red and still
merge-blocking is a required check that is never reported: it sits 'Expected —
waiting' (pending).
So build-gate is now SKIPPED via a job-level if: for exactly the successful
compile-only case. A required check whose job never starts stays pending, so the
PR remains un-mergeable without a red X and the run is not marked failed. This
must be a job-level skip (job never starts): a job that runs and reports
conclusion 'skipped' counts as PASSED and would let the PR merge.
The guard is deliberately narrow — !(build.result == success && skip_tests ==
true) — so a compile-only build that FAILED still runs the gate and goes red,
and the trivial-change skip still runs and goes green (docs-only PRs are not
left pending forever). The now-unreachable compile-only branch in the gate body
is removed. Merge qualification for fork code continues to come from a
maintainer pushing a *-reviewed-for-build tag (full trusted build with tests).
compile-gate (advisory) still runs and goes green, giving a clear positive
compile signal beside the pending build-gate.
Assisted-By: Claude Opus 4.8
actions/checkout@v4, setup-java@v4 and upload-artifact@v4 run on the node20
runtime, which is EOL (Node 20 reached end-of-life on 2026-03-24) and emits a
deprecation warning on every run. Bump each to its current release, all of
which run on node24, and pin by full commit SHA (matching how the third-party
actions in this workflow are pinned):
- actions/checkout v4 -> v7.0.1 (3d3c42e5aac5ba805825da76410c181273ba90b1)
- actions/setup-java v4 -> v6.0.0 (dd06d9cba3e5552c54d9f8ea23572deb30010f7c)
- actions/upload-artifact v4 -> v7.0.1 (043fb46d1a93c77aae656e7c1c64a875d1fc6a0a)
checkout v5+ requires Actions Runner >= v2.327.1, which GitHub-hosted
ubuntu-latest satisfies. setup-java v6 is unaffected for the Temurin 8/17/25
toolchains used here. actions/upload-release-asset@v1 is intentionally left as
is: it is archived with no maintained successor and belongs to a separate
change, not this node20 sweep.
Assisted-By: Claude Opus 4.8
A submitted PR now triggers a build automatically, split by trust:
- Same-repo PR (branch pushed by a committer): full build with tests and
secrets — same trust level as a direct branch push, which already builds
with secrets. build-gate can go green with no manual tag.
- Fork PR: compile-only with NO secrets. GitHub withholds secrets from fork
pull_request runs at the event level, and a compile-only build needs none,
so this is both automatic and safe. build-gate stays red; the secret-bearing
full build of fork code remains gated behind a maintainer pushing a
*-reviewed-for-build tag.
Uses pull_request (not pull_request_target), so fork runs get a read-only
token and no secrets; with the secret env values already resolving to '' when
tests are skipped, fork code never meets a secret.
- pull_request trigger: types [opened, synchronize, reopened, ready_for_review]
with the same paths-ignore as push; no branches: filter (targets PRs into any
base branch).
- build job if:: draft PRs are excluded (build only once marked ready).
- Determine build mode: a fork pull_request (head.repo.full_name != the base
repo) sets skip_tests=true; same-repo PRs run the full build.
The conditional secret env, -t flag, Test Reports guard, and both gate jobs
already key on the computed skip_tests flag, so they need no change.
Assisted-By: Claude Opus 4.8
A maintainer reviews a fork PR's exact SHA, then pushes a tag at that SHA:
'*-reviewed-for-compile' runs a compile-only build with NO secrets (cheap
sanity check), '*-reviewed-for-build' runs the full build with tests and
secrets. A tag push is a trusted push event in the base repo, so it receives
secrets — the only way to run secret-bearing validation on fork-authored code.
- push trigger: keep branches: ['**'] (required so the new tags: filter does
not silently disable branch triggers) and add the two reviewed-tag families.
- build job: a 'Determine build mode' step computes skip_tests (true for a
-for-compile tag or a workflow_dispatch skip_tests=true) and exposes it as a
job output. The five test-runtime secrets resolve to '' when skip_tests is
true, so their values never enter the environment on a compile-only run; -t
is passed on the same flag. Test Reports step is skipped when tests are.
- build-gate (required check): green only for a genuine full build with tests,
or a trivial-change skip; a compile-only success fails it. This also fixes a
pre-existing bug where a skip_tests=true dispatch satisfied the gate.
- compile-gate (new, advisory): green whenever the build compiled, regardless
of tests, giving -for-compile runs a distinct check without touching the
required build-gate.
Assisted-By: Claude Opus 4.8
Upstream committers have more freedom handling PRs, so the pull_request_review
trigger, the privileged-pr-build environment gate, and the associated
head-SHA / persist-credentials / base-ref conditionals are not needed here.
Remove them, keeping the plain push/workflow_dispatch build.
The dorny/paths-filter correctness fix (v4.0.3 + some-with-excludes + a
positive '**' rule) is retained: it fixes a pre-existing bug where docs-only
changes were misclassified as code changes on the push path too.
The downstream (SAP) branch keeps the environment-based approval approach via
a subsequent 'merge -s ours'.
Assisted-By: Claude Opus 4.8
v4 adds the 'some-with-excludes' predicate-quantifier, which is the natural
fit here: code is true only when a changed file matches the positive '**'
rule and none of the '!' exclusions. Replaces the v3.0.3 'every' workaround.
The only v4 breaking change is the Node 20->24 runtime bump.
Assisted-By: Claude Opus 4.8
The 'code' filter used only negated rules under the default 'some'
quantifier. picomatch treats '!README.md' as "any path except README.md",
so a changed README.md still matched a rule and set code=true, forcing a
full build for docs-only PRs (as seen on PR #48).
Add a positive '**' rule and set predicate-quantifier: 'every' (the pinned
v3.0.3 does not support the v4-only 'some-with-excludes'), so code is true
only when a changed file matches the '**' rule AND all '!' exclusions,
i.e. it is a non-trivial file.
Assisted-By: Claude Opus 4.8
The changes job used dorny/paths-filter without an explicit base/ref, so on
a pull_request_review event (which, unlike pull_request, is not auto-detected
by the action) it diffed against the default branch with a shallow clone and
treated every file as changed. A docs-only PR therefore ran a full build and
hit the deployment-review gate needlessly.
Diff the PR head (github.event.pull_request.head.sha) against the PR base
(github.event.pull_request.base.sha) explicitly, fetch full history so the
merge-base resolves, and grant pull-requests: read. Push and workflow_dispatch
behavior is unchanged (base/ref fall back to empty, fetch-depth to 1).
Assisted-By: Claude Opus 4.8
Add a pull_request_review (submitted) trigger and gate the changes/build
jobs on an approving review, so a PR (possibly from an untrusted fork)
never builds with secrets before a maintainer has vetted the exact code.
Secrets are released via a conditional 'privileged-pr-build' environment
referenced only on review runs; on push/workflow_dispatch the environment
name is empty (no gate), keeping existing behavior unchanged. The
environment holds no secrets, so repository-level values are used via
environment>repository precedence and rotation stays in one place.
Review runs check out the PR head SHA with persist-credentials: false so
untrusted build code cannot scrape the token. build-gate now treats a
skipped changes job (non-approving review) as pass rather than failure.
Assisted-By: Claude Opus 4.8
The local / master-SNAPSHOT / master-latest / 19/08/2026 09:58 update on
their yb.tl site may have broken things for us. E-mail to their support
address has been sent, and we need to wait until we receive
clarification.
In POLYCHROMATIC mode, Colorline.setPath(...) built zero polylines for a
single-vertex path, so getLength() returned 0 instead of 1. This broke the
FixesAndTails invariant tail.getLength() == lastShownFix - firstShownFix + 1,
after which mergeFixes computed out-of-range tail indices and threw an
IndexOutOfBoundsException while scrubbing the time slider.
Represent a one-vertex path as a single Polyline holding that vertex, matching
the interim single-vertex state insertAt builds up and getLength's case 1
already expects.
Assisted-By: Claude (claude-4.8-opus)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up to holding PolarDataMiner's race-keyed collections weakly. The weak
key on callbacksWaitingForFixIngestion was defeated by its own values: a parked
callback (the scheduleWindEstimationInstallation closure) strongly captures the
same TrackedRace that keys its entry, so the entry -- and the race with all its
tracks -- would never be collected while parked. That is a brittle contract to
place on callers (they must ensure their Runnable holds no strong reference to
the race), and easy to get wrong.
Instead:
- callbacksWaitingForFixIngestion goes back to a strong HashMap (weak keys were
misleading there) and is pruned explicitly.
- racesWithIngestedFixes stays a weak set (it has no values, so weak keys are
effective) and is additionally pruned eagerly as belt-and-suspenders.
- New PolarDataService.raceRemoved(TrackedRace) (delegating to
PolarDataMiner.raceRemoved) forgets both collections for a race.
- RacingEventServiceImpl.RaceAdditionListener.raceRemoved now calls
polarDataService.raceRemoved(trackedRace) (null-guarded, since the service
arrives via an OSGi ServiceTracker).
The runWhenPolarLoadingFinishedFor Javadoc gains an explicit LEAK CONTRACT
paragraph documenting that callers must arrange for raceRemoved to be called on
race removal, and why the map cannot rely on weak keys. Dropping a removed
instance's parked install callback loses nothing: a re-added race is a new
TrackedRace instance that gets a fresh raceAdded -> scheduleWindEstimationInstallation
-> runWhenPolarLoadingFinishedFor registration. Post-startup manual loads still
wait for the loading-pipeline drain (Gate A on racesWithIngestedFixes ensures the
race's own fixes are ingested before the callback is eligible; the drain then
covers it and any concurrently-loading races). Live TRACKING fixes flow through
the separate preFilteringProcessor chain and never perturb the loading drain.
Also documents the bounded, self-clearing lifecycle of
callbacksWaitingForLoadingOfAllRacesToRestoreToStart.
racesWithIngestedFixes was a HashSet<TrackedRace> that was only ever added to
(in raceFinishedLoading), never removed. Every race ever loaded stayed in the
set for the lifetime of the miner -- i.e. the server process -- pinning the
TrackedRace and, transitively, all of its tracks. On a long-running ARCHIVE
server that loads tens of thousands of races (and reloads across the day), this
is a real, unbounded memory leak; removed races could never be freed.
There is no safe explicit removal point: a runWhenPolarLoadingFinishedFor call
for a race may legitimately arrive long after its fixes were ingested (e.g. a
wind-estimation factory-service swap reschedules the install per race via
scheduleWindEstimationInstallation), so the set cannot be cleared on drain
without risking a stranded, never-firing callback for a still-live race.
Instead hold the set weakly: Collections.newSetFromMap(new WeakHashMap<>()).
Entries evict once a race is no longer strongly reachable elsewhere, at which
point no further runWhenPolarLoadingFinishedFor call for it is possible, so
losing the 'already ingested' bit is harmless. Live races keep their entry and
the ingestion gate behaves exactly as before. All access remains under the
existing 'synchronized (callbacksWaitingForFixIngestion)' monitor, so the
non-thread-safe weak collections are safe.
callbacksWaitingForFixIngestion (also keyed by TrackedRace) is switched to a
WeakHashMap for the same reason: a race removed while still parked here (e.g.
removed while stuck in LOADING before ingestion) would otherwise be pinned
forever. Its entries for live races are still removed explicitly in
raceFinishedLoading.
Javadoc on both fields rewrapped to the project's 120-column comment width.
Follow-up to a960398cd3, which inadvertently captured a half-edited state of
this file (the previous commit staged it while a local edit was in flight).
This restores the intended version: 'information'/'sites'/'Internet Explorer'
wording and the empty-line cleanup in the web-driver system-properties comment.
Comment/whitespace only; no test behavior change.
The Javadoc on PolarDataService.runWhenPolarLoadingFinishedFor (interface),
PolarDataMiner (impl) and the RacingEventServiceImpl.scheduleWindEstimationInstallation
caller all read as if the callback fired once the fixes belonging to the given
race had drained. In reality the loading pipeline (preFilteringProcessorForLoadedFixes)
is shared across all races and has no per-race completion tracking, so the
callback fires only on a global drain: when the fixes of every race ingested so
far have been processed.
The race parameter does not scope the wait; it only gates WHEN the callback may
attach to the shared drain, via two conditions that must both hold: (1) the
race's own fixes have been ingested (so the callback can't fire on a pipeline
that is merely momentarily idle before this race was queued), and (2)
markLoadingOfAllRacesToRestoreStarted() has been signalled (so an idle window
between two startup races' ingestion bursts isn't mistaken for a real drain).
This global-wait behavior is deliberate: we want the polar model to reflect all
loaded data before any maneuver-based wind estimation is installed, and we accept
the resulting startup sequentiality (an early race's estimator install waits
behind the whole loaded-fix backlog). Documentation only; no behavior change.
The method name is kept as-is.
The delegate's canBeUpdated() was returning true unconditionally, hiding the
distinction between the DB-load state (cacheToUse = ManeuversFromDatabase,
inner canBeUpdated=false) and the compute state (cacheToUse =
ManeuversFromSmartFutureCache, inner canBeUpdated=true). That broke the
gate on TrackedRaceImpl.feedAlreadyKnownManeuversToWindEstimation which
uses maneuverCache.canBeUpdated() to decide whether the current JVM's
cache is DB-backed and therefore needs alreadyClassifiedManeuversAvailable
to feed the estimator.
With the pre-fix behavior, the gate never opened even for Path A races:
feed was always skipped, the estimator never received the loaded
maneuvers, PreClassifiedUpdate never ran, and the MBE wind track stayed
empty. Observable as hasWindData=false on all fingerprint-matched races
of "The Final Push 49erFX" (W1..W9, W11, W12) on archive-candidate even
28+ hours after the wind-estimation install callback had fired, while
production (running older code) had hasWindData=true.
Change canBeUpdated() to delegate to cacheToUse.canBeUpdated(). Now it
truthfully reflects which inner cache is in use. All other callers of
cacheToUse.canBeUpdated() within the delegate (ensureFilled, computeAndStore,
triggerUpdate) already read the inner cache directly and are unaffected.
The only external caller was TrackedRaceImpl.feedAlreadyKnownManeuversToWindEstimation
which now correctly opens the gate for Path A and keeps it closed for
Path B.
Javadoc on the method explains the semantics and how it toggles across
resume() and triggerUpdate() calls.
The existing isAllEmpty() self-heal in ManeuverCacheDelegate.resume() only
caught the case where every competitor's persisted maneuver list was null
or literally empty. A separate poisoned state slipped through: lists are
non-empty but every stored maneuver is typed as UNKNOWN (or PENALTY_CIRCLE),
which happens when the detector ran while getWind(pos, at) still returned
null everywhere and createManeuverFromManeuverCurveAndWind hit its 'wind ==
null' fall-through in ManeuverDetectorImpl.
Feeding such records to the estimator via alreadyClassifiedManeuversAvailable
produces zero wind fixes because
IncrementalMstHmmWindEstimationForTrackedRace.mapManeuverType maps UNKNOWN
and PENALTY_CIRCLE to null and the whole batch gets filtered out. Observable
as hasWindData=false on the affected races even after the wind estimation
callback has fired. Confirmed on archive-candidate for W10 of "The Final
Push 49erFX" (982 stored maneuvers, all UNKNOWN) and for Abeam Training
2016 Race 1 (453 stored, all UNKNOWN); production shows the same event with
hasWindData=true because it ran with older code paths that populated the
MBE track through a different route.
The fix adds a companion check, hasNoClassifiableManeuver, invoked after
isAllEmpty in resume(). It returns true iff no stored maneuver anywhere in
the loaded map is typed as TACK/JIBE/HEAD_UP/BEAR_AWAY (the four types
mapManeuverType turns into a non-null ManeuverTypeForClassification). When
it fires, resume() logs the reason and falls through to the compute path,
which now correctly sequences its storage after the wind estimator has
settled (see computeAndStore / retypeAndStoreAfterWindEstimationSettled) so
the recompute produces properly-typed maneuvers and overwrites the poison.
isClassifiableType() is a tiny helper listing the four classifiable types;
a comment reminds future maintainers to keep it in sync with mapManeuverType.
Restores the compute-path guard on feedAlreadyKnownManeuversToWindEstimation.
Without the gate, setWindEstimation submits per-competitor feed tasks that
call maneuverCache.get(waitForLatest=true) concurrently with the ordinary
detector-driven feed via newManeuverSpotsDetected. Depending on ordering the
feed task either sees the recalculation already in progress (blocks, then
double-feeds via alreadyClassifiedManeuversAvailable, whose per-competitor
PreClassifiedUpdate reconciliation clobbers the graph path's whole-race
track) or completes before the trigger fires (get returns null, feed skips).
The double-feed shows up as flakiness of
IncrementalMstHmmWindEstimationForTrackedRaceTest -- observed locally with a
ratio-of-matching-fixes of 0.658 vs the 0.75 threshold, and also seen on
GitHub Actions run 30262443673.
The rationale for removing the gate previously (that after the bug6241
polar-data hold-back the estimator may be installed after detection ran
with a null WindEstimationInteraction) is now handled explicitly by the
retype-and-store choreography in ManeuverCacheDelegate.computeAndStore:
its runWhenWindEstimationInstalled callback re-drives the detector once
the estimator is in place, and freshly built detectors (whose per-competitor
cache was cleared by updateManeuversAndWindWithNewWindEstimation at install
time) emit spots through the graph path with cross-competitor aggregation
intact -- no per-competitor slice ever needed.
Adds a substantial comment on the method spelling out why the gate must
stay in place, so future edits don't remove it again.
The previous single-round retypeAndStoreAfterWindEstimationSettled was
incorrect: it called waitUntilDone on the estimator before any spots
had been re-emitted (the compute-path's initial detection ran with a
null WindEstimationInteraction because the estimator hadn't been
installed yet, so no spots were pushed). waitUntilDone then returned
immediately on an empty queue, and the subsequent recalculate emitted
spots but returned before the estimator could process them and produce
wind fixes. Net effect: stored maneuvers were still typed with null
wind (mostly UNKNOWN), reproducing the poisoned-DB pattern the fix was
supposed to prevent.
Split into two rounds:
1. First recalculate + get per competitor. Since setWindEstimation
cleared maneuverDetectorPerCompetitorCache, this builds a fresh
detector with a non-null WindEstimationInteraction, runs a full
scan, and *does* emit spots to the estimator during this pass.
2. waitUntilDone -- now the estimator's queue has the spots from
round 1 and this actually waits for the graph inference to
produce wind fixes and flow them into the tracked race's
MANEUVER_BASED_ESTIMATION track.
3. Second recalculate + get per competitor. The detector's
lastManeuverDetectionResult is now populated and no new raw fixes
have arrived, so it takes the re-type branch of detectManeuverSpots
-- which routes through determineManeuversFromManeuverCurve and
types the spots using trackedRace.getWind(...), which now sees the
estimator's fixes (bug6274 removed the MANEUVER_BASED_ESTIMATION
exclusion). The isManeuverSpotWindNearlySame check correctly
detects that the previous typing had null wind and the current
has non-null, so it doesn't short-circuit to the UNKNOWN spot.
4. Snapshot the classified maneuvers and store.
For W1 in 49erFX (5 competitors), pre-fix Mongo had 1 competitor with
362 typed maneuvers (whose detection happened to run after
setWindEstimation) and 1 with 628 all-UNKNOWN (whose detection ran
before setWindEstimation). Post-fix, both should be typed.
The Runnable passed to runWhenWindEstimationInstalled(...) fires
synchronously on whichever thread first invokes setWindEstimation with
a non-null argument. That thread is typically a shared
background-executor thread (the same pool used by SmartFutureCache and
the wind estimator's GraphRecalculationTask). Running waitUntilDone,
recalculate(...), and get(waitForLatest=true) on that thread makes the
callback block waiting for tasks that need the same pool to progress,
which starves the pool -- fatal with pool size 1, and observed as
timeout retries on saturated pools in local runs.
Detach the callback body onto a dedicated Thread so the setWindEstimation
caller returns immediately and the shared pool stays free to service the
tasks the retype-and-store step is waiting on.
The previous ManeuverCacheDelegate.resume() Path B stored the initial
detector output before the wind estimator had a chance to produce any
fixes. For races without another wind source (typically no-race-start
practice sessions with only GPS tracks), that first pass typed every
spot as UNKNOWN or produced no maneuvers at all, and the empty result
was persisted. On subsequent server starts the fingerprint matched, an
empty maneuver list got loaded, feedAlreadyKnownManeuversToWindEstimation
had nothing to hand to the estimator, and no wind fixes materialised.
Two fixes:
* New primitive TrackedRace.runWhenWindEstimationInstalled(Runnable),
default fires synchronously if getWindEstimation() is already
non-null else no-op (suitable for mocks). TrackedRaceImpl overrides
with the full callback-registry pattern used by runWhenPastLoading
(two-cooperating-listeners, CAS-guarded single fire, race-removal
cancellation via regatta listener). Two new transient fields on
TrackedRaceImpl for the callback list and its monitor, both
reinitialized in readObject for replicas.
* ManeuverCacheDelegate.resume() refactored into three methods --
resume(), computeAndStore(), retypeAndStoreAfterWindEstimationSettled().
The compute path now runs the detector, then via
runWhenWindEstimationInstalled(...) waits until the estimator is
installed, awaits its inference via waitUntilDone(), recalculates
each competitor so maneuvers are re-typed using the estimator's
MANEUVER_BASED_ESTIMATION fixes (visible to typing thanks to bug6274),
then snapshots and persists. The DB now captures the converged
classification, not the pre-estimator empty state.
resume() also self-heals against DB records poisoned by earlier server
runs: on fingerprint match with all-empty stored maneuvers, treats as
a stale cache miss and falls through to compute. Races that legitimately
have no maneuvers (short races with no motion) will recompute on every
startup, which is acceptable because the cost is proportional to zero.
Axel Uhl
