Following up on my -1 with fixes rather than just complaints. Two PRs,
both against main:
#868 -- the blocker plus two papercuts
- LICENSE-wheel now lists all 130 npm packages compiled into the
tracking UI bundle (MIT 115, ISC 9, Apache-2.0 3, BSD-3-Clause 2,
PSF 1), generated from the build sourcemap rather than by hand, so
it cannot silently drift. NOTICE unchanged -- none of the 130 ships
a NOTICE file.
- release-validation.yml gains a --check step. RAT excludes
burr/tracking/server/build/**, so nothing was inspecting the only
content the wheel adds over the sdist; now something is.
- The botocore-above-importorskip fix, so `pytest tests/` runs for
voters without the [bedrock] extra.
- /api/v0/version off pkg_resources and onto importlib.metadata.
All 29 checks green.
#869 -- the npm advisories, stacked on #868
16 advisories down to 2. The one that actually mattered was
mdast-util-to-hast (unsanitized class attribute, 28 modules in the
bundle, reached via react-markdown when it renders trace and chat
content). The rest were build-time only.
The 2 remaining are react-router, still in range at 7.18.2; the only
fix is a major bump to 8.3.0. The advisory is about RSC mode and the
UI is a static client-side SPA with no RSC or server actions, so it
isn't reachable -- I'd suggest doing that upgrade deliberately rather
than as part of a release repair, but it is the PPMC's call.
Two things I found while fixing the above that were not in my original
review:
1. The UI test suite has never run in CI. ui.yml builds, lints and
formats but never invokes vitest, so App.test.tsx -- the only UI
test -- executes nowhere, and it has been failing on newer Node with
no signal. Fixed in the second PR.
2. The cause of that failure is a real robustness bug, not a test
artifact: useTheme accessed window.localStorage behind only a
`typeof window` guard. Where storage is unreachable -- sandboxed
iframe, storage disabled, or Node's own localStorage global
shadowing jsdom's -- it throws during the initial useState and takes
down the entire App render, not just the theme. Also fixed there.
Still open from my review, needing a PPMC decision rather than a patch:
website/public/logos/taskhuman.svg is a third-party company mark in the
voted source archive with no recorded provenance or permission, and RAT
skips it because **/*.svg is excluded. Someone who knows the history
should confirm permission exists, or it should come out. Happy to file
an issue if that is easier to track.
If both land I am glad to re-verify an RC1 quickly -- the whole check is
scripted on my end now, including a byte-for-byte rebuild of the wheel's
UI bundle from the source tarball.
Jarek Potiuk