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

Reply via email to