Skip to content

Port the public site onto the Go core, and fix a listener data race - #3

Merged
Paulkm2006 merged 5 commits into
refactor/walisfrom
site/port-onto-walis
Aug 3, 2026
Merged

Port the public site onto the Go core, and fix a listener data race#3
Paulkm2006 merged 5 commits into
refactor/walisfrom
site/port-onto-walis

Conversation

@yujiezhang-ops

@yujiezhang-ops yujiezhang-ops commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Brings the public site work from main onto refactor/walis, adapted to this branch's data sources. refactor/walis was branched before that work landed, so its site/ directory is file-for-file the 2026-07-29 version — none of the 23 new files existed here.

Base is refactor/walis, not main, so this does not ask you to resolve the two parallel Go migrations. That decision stays with PR #2.

The site port

The i18n routing layer, five English pages, the compatibility explorer, the self-playing activation demo, and the theme/locale controls.

main fed the site from src/generated/*.json, produced by Python scripts this branch removes. So the port had to change the data layer, not just move files: catalog.ts now reads agents.lock.json directly (gaining command, configPath and groups, which the explorer and demo display), and a new release-channel.ts maps a GitHub release into the channel shape the download and security pages already consume. Eleven as unknown as / as any casts are gone with the untyped JSON imports they existed to paper over.

catalog.ts also imports its types from explorer.ts instead of redeclaring them — the duplicate declarations were how a new protocol could reach the pages while the explorer still called it unsupported.

A data race in the installer

go test -race -cover ./... failed on internal/process, and it turned out not to be a test artifact.

exec.Cmd copies stdout and stderr on separate goroutines, so the listener passed to RunWithOutput was entered from both at once. The install runtime hands it a closure that redacts and forwards (internal/app/install.go:373), holding no lock. That is a production data race, and it triggers whenever a command writes to both streams — npm does on every install.

The lock now lives in the runner: "calls are serialised" is the contract a listener should be able to rely on, and both streamWriters share one mutex (a mutex per writer would leave the two goroutines taking different locks and entering the listener together anyway).

The existing tests only caught this by accident. They left stderr empty, so -cover adding a GOCOVERDIR not set warning to the child's stderr was what exposed the race at all — and that same warning polluted three assertions about captured output. The helper now gets its own scratch GOCOVERDIR, and TestOSRunnerSerialisesListenerAcrossStreams drives 50 interleaved writes per stream through an unlocked listener. Reverting the fix makes it fail with DATA RACE; I checked.

A fresh clone could not build

manifest_embed.go says a checked-in .keep makes the stage-0 shell buildable before Vite runs. The file was never in the repository: two dist/ rules excluded the directory itself, and excluding a directory stops git descending into it, so the !frontend/dist/.keep exception already sitting in the root .gitignore could never take effect. go:embed all:frontend/dist therefore had nothing to embed, and go vet ./... failed on a fresh clone until someone happened to build the frontend first. Verified by cloning this branch and running go vet ./... with no frontend build — it now passes, and build output stays ignored.

Three judgement calls in the site copy

The security pages asserted something the data cannot support. They claimed a specific macOS arm64 build passed a cleanroom review; the release feed carries no build provenance. They now state what is checkable against the release page — channel, version, platform, digest — and say the provenance conclusions are recorded by the release process rather than asserted by the page. release-channel.ts reports native_build: false and cleanroom: "not-recorded" for the same reason: those are the honest readings of "the feed cannot tell us", and asserting true would put a verification badge on the site that nothing checked.

A verification gate lost its subject. validate-build.mjs used to re-hash every artifact in dist/downloads/ against the digest the release index claimed, so a page could not print a checksum the file did not have. With artifacts on GitHub Releases there is no local file to hash. Removed, with a comment recording the cost: a wrong digest from the release feed now reaches the page unchallenged. Restoring an equivalent check means fetching and hashing each asset during validation.

npm run build was failing after reporting success — it ended in npm run validate, but validate had been deleted from package.json. Restored.

Gates

go vet ./...                      clean
go test -race -cover ./...        all packages ok, 0 failures
frontend: npm run build           ok (tsc --noEmit clean)
site: npm test                    40 passed
site: npm run build               0 type errors, 32 pages, validate-build passed
site: npx playwright test         133 passed, 23 skipped, 0 failed (3 viewports)

Two environment notes for whoever runs this next:

  • go.mod requires Go 1.26.5 and proxy.golang.org is unreachable from here, so the automatic toolchain download fails. I installed 1.26.5 from dl.google.com (checksum verified against go.dev/dl) and used GOPROXY=https://goproxy.cn with GOSUMDB=sum.golang.google.cn. go.sum is unchanged — no hash was substituted, and the existing entries were sufficient.
  • GITHUB_TOKEN is effectively required for the site build. Unauthenticated GitHub API access is 60 requests/hour and a 32-page build exceeds it; over the limit the build fails outright rather than degrading.

The 23 skips are 8 pre-existing viewport skips plus 15 because this repository has no published releasesgetLatestRelease() returns null, so the download page renders its "not published yet" notice and the platform-picker tests have nothing to drive. They skip with a stated reason rather than being deleted, so they still catch a regression once artifacts exist. PR #2's own baseline behaves the same way; I checked before changing anything.

🤖 Generated with Claude Code

yujiezhang-ops and others added 5 commits August 3, 2026 11:19
The site pages were written against two generated JSON files that a Python
step produced from the release artifacts. Those scripts are gone, so the
pages need the same shapes from what remains: agents.lock.json for the
catalog, and the GitHub Releases API for the downloads.

catalog.ts gains `command`, `configPath` and `groups`, which the explorer
and the activation demo already display, and now declares its types by
importing explorer.ts rather than redeclaring them. Re-declaring was how a
new protocol could reach the pages while the explorer still called it
unsupported.

release-channel.ts is new. It maps a release into the channel shape the
download and security pages consume, and reports what the API cannot tell
it as unknown instead of filling it in: `native_build: false` and
`cleanroom: "not-recorded"` are the honest readings of "the release feed
does not carry build provenance". Asserting `true` here would put a
verification badge on the site that nothing checked.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports the public site work from main: the i18n routing layer with its
hreflang and canonical handling, five English pages, the compatibility
explorer, the activation demo that plays itself once scrolled into view,
and the theme and locale controls.

Every page that read a generated JSON file now reads the typed catalog and
release-channel modules instead, which also removes eleven `as unknown as`
and `as any` casts that only existed to give an untyped JSON import a
shape.

The security pages needed more than a rewire. They asserted that a
specific macOS arm64 build had passed a cleanroom review — a claim the
release feed cannot support. They now state what is checkable against the
release page (channel, version, platform, digest) and say plainly that the
build-provenance conclusions are recorded by the release process rather
than asserted here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`npm run build` ended in `npm run validate`, but the validate script had
been removed from package.json — so every build failed on a missing script
after reporting success. Restored.

validate-build.mjs used to re-hash each artifact in dist/downloads/ and
compare it against the digest the release index claimed, which meant a page
could not print a checksum the file did not have. That check has no subject
now: the site links to GitHub Releases instead of hosting the artifacts, so
there is no local file to hash. Removed, with a comment recording the cost —
a wrong digest from the feed now reaches the page unchallenged — and what
restoring an equivalent gate would take. release-index.json also came off
the required-outputs list, since the site no longer emits it.

The download-page tests skip, with a stated reason, when no release is
published: the picker they drive is replaced by the unavailable notice, and
this repository's release feed is currently empty. Skipping keeps them
around to catch a regression once artifacts exist, where deleting them
would let the download flow rot unnoticed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
exec.Cmd copies stdout and stderr on separate goroutines, so the listener
passed to RunWithOutput was entered from both at once. The install runtime
hands it a closure that redacts and forwards (internal/app/install.go), and
that closure holds no lock — so this was a data race in production, not only
under test. It shows up whenever a command writes to both streams, which npm
does on every install.

The lock lives in the runner rather than in each caller: "calls are
serialised" is the contract a listener should be able to rely on. Both
streamWriters share one mutex, since a mutex per writer would have the two
goroutines taking different locks and entering the listener together anyway.

TestOSRunnerSerialisesListenerAcrossStreams drives 50 interleaved writes to
each stream through a listener that appends without locking, mirroring the
production shape. Reverting the fix makes it fail with DATA RACE, which the
previous tests only did by accident: they left stderr empty, so `-cover`
adding a "GOCOVERDIR not set" warning was what exposed the race at all. That
warning also polluted three assertions about captured output, so the helper
now gets a scratch GOCOVERDIR of its own.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
manifest_embed.go says a checked-in .keep makes the stage-0 shell buildable
before Vite has produced dist. The file was never in the repository: two
`dist/` rules excluded the directory itself, and excluding a directory stops
git descending into it, so the `!frontend/dist/.keep` exception that was
already sitting in the root .gitignore could never take effect.

The consequence is that `go:embed all:frontend/dist` had nothing to embed in
a fresh clone, and `go vet ./...` and `go build` failed on a missing embed
pattern until someone happened to build the frontend first.

Both rules now list their entries (`dist/*` plus the negation) instead of
excluding the directory, and the root rule is anchored to `/dist/` so it stops
matching at arbitrary depth. Build output stays ignored — verified that
frontend/dist/index.html and site/dist/ still are.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yujiezhang-ops yujiezhang-ops changed the title Port the public site onto the Go core, reading releases from GitHub Port the public site onto the Go core, and fix a listener data race Aug 3, 2026
@Paulkm2006
Paulkm2006 merged commit 2eb4f61 into refactor/walis Aug 3, 2026
@Paulkm2006
Paulkm2006 deleted the site/port-onto-walis branch August 5, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants