build: the lane was green about files it never compiled — fix + two guards #60

Merged

triform-admin merged 12 commits from c8/three-unbuilt-tests into main 2026-07-31 08:44:45 +00:00

Conversation 5 Commits 12 Files changed 10 +915 -7

triform-admin commented 2026-07-30 21:24:17 +00:00

Owner

Copy link

What this is

Three commits, one thread: the x264-t7 build lane was green about a smaller
set of files than anyone believed it covered, and this makes that impossible
to repeat.

1. Add the three test targets nothing had compiled (cd1b3b6)

capture/**/BUILD.gn declares seven test() targets. Three of them
(adm, input_dispatch, pointer_state) appeared in zero build
manifests. They had been declared, reviewed, merged, and never compiled by
anything for roughly ten weeks.

2. The first compile found a real defect (9b22f70)

Adding them was not a formality. The very first build failed:

cb_audio_device_module_test.cc:244:29: error: variable type
'test::CbAudioTestRecorder' is an abstract class
audio_device_defines.h:81:16: note: unimplemented pure virtual method
'PullRenderData' in 'CbAudioTestRecorder'

CbAudioTestRecorder declared itself public webrtc::AudioTransport and
implemented two of that interface's three pure virtuals. The class was
abstract, so the two test::CbAudioTestRecorder recorder; declarations
could not compile — and nothing had ever tried.

The header's own TODO(M55-R2-test-audio-device-header) predicted this down
to the mechanism: "confirm the same header still satisfies AudioTransport in
this branch when the test first goes through the chromium build pod." It did
not, and the first trip through the build pod is exactly what found it.

The fix is a silence-renderer, same contract and same rationale as the
existing NeedMorePlayData — M5.5's measurement is capture-side, so the
render pull only has to be safe. One trap earned a comment: PullRenderData
reports geometry in bits per sample and frames, where
NeedMorePlayData uses bytes and samples. Copying the sibling's arithmetic
would memset the wrong length.

3. Two guards so neither half recurs (9131b30)

tools/lint/build_targets_lint.py — every test() target must be built
by at least one lane. Fails in both directions: declared-but-unbuilt is the
ten-week gap; built-but-undeclared is a typo or rename that gn would reject
hours into a job, caught in 40ms instead.

The rule is deliberately "at least ONE lane", not "every lane" —
pcf_unittests is x264-t2-only on purpose and the encoder tests are
profile-gated. Demanding uniformity would produce exactly the false positives
CLAUDE.md warns cost more trust than a missed defect costs time.

Two things worth calling out about the self-tests:

• One case asserts that a target named only inside a manifest's comment
  block does not count as built. The real manifests document all seven in
  comments, so a parser that scooped those up would pass on the exact tree
  this lint exists to fail — green, sitting on top of the gap.
• Writing the tests changed the code. Testing a dropped trailing "s"
  (_unittest vs _unittests) showed direction 2 skipping the likeliest
  typo of all, because the predicate was a suffix match. Now a substring, so
  both arms fire together on a misspelling and the reader sees a near-miss
  pair rather than hunting for a manifest entry that is sitting right there.

infra/k8s/chromeless-build/fire-build.sh — the other half is not
visible to a repo lint, because the divergence is between a file and a
cluster object. Firing a build by hand means copying a manifest, adjusting
name/ref/targets, applying; every step can silently desync, and the Job runs
to completion either way, reporting a verdict about whatever the live spec
said.

That cost three wrong results in one afternoon: a manifest sed-derived from
a previous job's copy (branch had six targets, live Job had three, reported
as "all six compile"); the same class again; and a job still pinned to
OUR_REPO_REF=main while the commit under test was on a branch.

The fix in all three was identical — read the live object back — so the
script does that before the job burns 30 minutes rather than after. Manifest
comes from git show <ref>:<path>. OUR_REPO_REF is rewritten at both sites
and it aborts on fewer than two (an init/build mismatch silently runs an old
build script; cost two attempts on 2026-05-17). Post-apply, live targets and
ref are diffed against the ref's manifest; mismatch deletes the Job and exits
non-zero. The ref argument is mandatory — defaulting to main is precisely
how you test main while believing you tested your branch.

Verification

make verify clean, self-tests pass. The guards were tested against the live
cluster, not just read:

• injected a manifest/live divergence → guard named the missing target,
  deleted the Job, exited 1
• happy path → verified nine targets against the ref
• both refusal paths (no ref, bad ref) → exit 1
• reverted the manifest to its real pre-fix state → lint reproduced the
  ten-week gap and named all three targets

The tampered-run Job was removed and the in-flight build was left untouched.

Status of the C++

PullRenderData is compile-verified only in the negative: the t7 lane
produced the error this fixes. The fix itself is in a build that is still
running as of this writing (chromeless-build-c8c, NINJA_KEEP_GOING=0 so
one pass collects every remaining drift error rather than one per cycle).
Two of the three newly-added targets have never been compiled by anything, so
further drift in them is likely — that result will be posted here.

## What this is Three commits, one thread: the x264-t7 build lane was green about a smaller set of files than anyone believed it covered, and this makes that impossible to repeat. ### 1. Add the three test targets nothing had compiled (`cd1b3b6`) `capture/**/BUILD.gn` declares seven `test()` targets. Three of them (`adm`, `input_dispatch`, `pointer_state`) appeared in **zero** build manifests. They had been declared, reviewed, merged, and never compiled by anything for roughly ten weeks. ### 2. The first compile found a real defect (`9b22f70`) Adding them was not a formality. The very first build failed: ``` cb_audio_device_module_test.cc:244:29: error: variable type 'test::CbAudioTestRecorder' is an abstract class audio_device_defines.h:81:16: note: unimplemented pure virtual method 'PullRenderData' in 'CbAudioTestRecorder' ``` `CbAudioTestRecorder` declared itself `public webrtc::AudioTransport` and implemented two of that interface's three pure virtuals. The class was abstract, so the two `test::CbAudioTestRecorder recorder;` declarations could not compile — and nothing had ever tried. The header's own `TODO(M55-R2-test-audio-device-header)` predicted this down to the mechanism: *"confirm the same header still satisfies AudioTransport in this branch when the test first goes through the chromium build pod."* It did not, and the first trip through the build pod is exactly what found it. The fix is a silence-renderer, same contract and same rationale as the existing `NeedMorePlayData` — M5.5's measurement is capture-side, so the render pull only has to be safe. One trap earned a comment: `PullRenderData` reports geometry in **bits** per sample and **frames**, where `NeedMorePlayData` uses bytes and samples. Copying the sibling's arithmetic would `memset` the wrong length. ### 3. Two guards so neither half recurs (`9131b30`) **`tools/lint/build_targets_lint.py`** — every `test()` target must be built by at least one lane. Fails in both directions: declared-but-unbuilt is the ten-week gap; built-but-undeclared is a typo or rename that `gn` would reject hours into a job, caught in 40ms instead. The rule is deliberately "at least ONE lane", not "every lane" — `pcf_unittests` is x264-t2-only on purpose and the encoder tests are profile-gated. Demanding uniformity would produce exactly the false positives CLAUDE.md warns cost more trust than a missed defect costs time. Two things worth calling out about the self-tests: - One case asserts that a target named only inside a manifest's **comment block** does not count as built. The real manifests document all seven in comments, so a parser that scooped those up would pass on the exact tree this lint exists to fail — green, sitting on top of the gap. - Writing the tests changed the code. Testing a dropped trailing "s" (`_unittest` vs `_unittests`) showed direction 2 skipping the likeliest typo of all, because the predicate was a suffix match. Now a substring, so both arms fire together on a misspelling and the reader sees a near-miss pair rather than hunting for a manifest entry that is sitting right there. **`infra/k8s/chromeless-build/fire-build.sh`** — the other half is not visible to a repo lint, because the divergence is between a file and a cluster object. Firing a build by hand means copying a manifest, adjusting name/ref/targets, applying; every step can silently desync, and the Job runs to completion either way, reporting a verdict about whatever the live spec said. That cost three wrong results in one afternoon: a manifest `sed`-derived from a previous job's copy (branch had six targets, live Job had three, reported as "all six compile"); the same class again; and a job still pinned to `OUR_REPO_REF=main` while the commit under test was on a branch. The fix in all three was identical — read the live object back — so the script does that before the job burns 30 minutes rather than after. Manifest comes from `git show <ref>:<path>`. `OUR_REPO_REF` is rewritten at both sites and it aborts on fewer than two (an init/build mismatch silently runs an old build script; cost two attempts on 2026-05-17). Post-apply, live targets and ref are diffed against the ref's manifest; mismatch deletes the Job and exits non-zero. The ref argument is mandatory — defaulting to `main` is precisely how you test `main` while believing you tested your branch. ## Verification `make verify` clean, self-tests pass. The guards were tested against the live cluster, not just read: - injected a manifest/live divergence → guard named the missing target, deleted the Job, exited 1 - happy path → verified nine targets against the ref - both refusal paths (no ref, bad ref) → exit 1 - reverted the manifest to its real pre-fix state → lint reproduced the ten-week gap and named all three targets The tampered-run Job was removed and the in-flight build was left untouched. ## Status of the C++ `PullRenderData` is **compile-verified only in the negative**: the t7 lane produced the error this fixes. The fix itself is in a build that is still running as of this writing (`chromeless-build-c8c`, `NINJA_KEEP_GOING=0` so one pass collects every remaining drift error rather than one per cycle). Two of the three newly-added targets have never been compiled by anything, so further drift in them is likely — that result will be posted here.

triform-admin added 4 commits 2026-07-30 21:24:17 +00:00

build: add the three test targets nothing has compiled in ~10 weeks ... cd1b3b609b

adm, input_dispatch, pointer_state are declared in the tree and absent from
every lane's CHROMELESS_BUILD_TARGETS. Deliberately left out of the earlier
targets change so bitrot would not be discovered while landing a codec fix.
This is that change.

Expectation, stated before the build runs so the result means something: the
//base public_deps defect hit 19 source_sets, and pointer_state AND
input_dispatch were both among them. Their test binaries are two of the
never-built five. Plausible that a previous attempt hit that wall and dropped
the target rather than the dep — unproven, but the shape fits. With the
public_deps fix in the base, they may now build. Or they may have drifted
against ~10 weeks of chromium API churn; this repo already carries three pure
drift-fix commits (JSONReader::ReadDict, raw_ptr<AudioDeviceModule>,
RtpTransceiverDirectionToString).

Either outcome is information. A clean build says the dep was the whole story;
compile errors give a concrete list of what a decade of unwatched drift costs.

Stacked on the verified base (public_deps + derived STEP 7 + C4) — a branch off
main cannot build these, which is the mistake that failed my first C5 attempt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(audio): implement PullRenderData — the test recorder was abstract ... 9b22f70567

CbAudioTestRecorder declared itself `public webrtc::AudioTransport` but
implemented only two of that interface's three pure virtuals. The third,
PullRenderData, was never written, so the class was abstract and the two
`test::CbAudioTestRecorder recorder;` declarations in
cb_audio_device_module_test.cc could not compile:

  cb_audio_device_module_test.cc:244:29: error: variable type
  'test::CbAudioTestRecorder' is an abstract class
  audio_device_defines.h:81:16: note: unimplemented pure virtual method
  'PullRenderData' in 'CbAudioTestRecorder'

This did not surface for ~10 weeks because no build ever compiled it.
cloud_browser_adm_unittests is a real gn target that nothing had put in
CHROMELESS_BUILD_TARGETS, so ninja never visited the TU. The lane was
green the whole time — it was green about a smaller set of files than
anyone thought it covered. Two sibling targets
(input_dispatch, pointer_state) were in the same state; this commit adds
all three to the x264-t7 lane so the gap cannot silently reopen.

The header's own TODO(M55-R2-test-audio-device-header) predicted this,
down to the mechanism: "confirm the same header still satisfies
AudioTransport in this branch when the test first goes through the
chromium build pod." It did not, and the first trip through the build
pod is what found it.

The implementation is a silence-renderer, same contract and same reason
as the existing NeedMorePlayData: M5.5's measurement is capture-side, so
the render pull only has to be safe, not meaningful. One trap worth the
comment it got — PullRenderData reports geometry in BITS per sample and
FRAMES, where NeedMorePlayData uses bytes and samples, so the memset
length is (frames * channels * bits/8). Copying the sibling's arithmetic
would zero the wrong number of bytes.

Compile-verified on the t7 build lane (this is the error that lane
produced); the fix itself is not yet compiled — the re-fire is next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

build: close the two gaps that let a green lane mean nothing ... 9131b30291

Both halves of the same defect — a build reporting success about a smaller
set of files than anyone believed it covered.

GAP 1: declared but never compiled (tools/lint/build_targets_lint.py)

capture/**/BUILD.gn declares seven test() targets. The k8s manifests each
name an explicit ninja target list. Nothing connected the two, so a target
could be declared, reviewed, merged, and never compiled by anything. Three
of the seven were in that state for ~10 weeks. When they were finally built
today the first compile failed immediately (CbAudioTestRecorder was
abstract), which is the whole point: the code was broken the entire time and
every lane was green.

The lint fails in both directions. Declared-but-unbuilt is the ten-week gap.
Built-but-undeclared is a typo or a rename that gn would reject hours into a
job — 40ms here instead.

The rule is "at least ONE lane builds it", not "every lane". pcf_unittests
is x264-t2-only on purpose and the encoder tests are profile-gated;
demanding uniformity would generate exactly the false positives CLAUDE.md
warns cost more trust than a missed defect costs time.

Self-tests build their own failure states, including a replay of the real
ten-week gap. One case is worth naming: a target mentioned only inside a
manifest's comment block must not count as built. The real manifests list
all seven in comments for documentation, so a parser that scooped those up
would pass on the exact tree this exists to fail — green, on top of the gap.

Writing the tests changed the code. Testing a dropped trailing "s"
(_unittest vs _unittests) showed direction 2 skipping the likeliest typo of
all, because the predicate was a suffix match. Widened to a substring: both
arms now fire together on a misspelling, so the reader sees a near-miss pair
instead of hunting for a manifest entry that is sitting right there.

GAP 2: the manifest you edited vs the Job that ran (fire-build.sh)

Not visible to a repo lint — the divergence is between a file and a cluster
object. Firing a build by hand means copying a manifest, adjusting
name/ref/targets, applying. Every step can silently desync, and the Job runs
to completion either way, reporting a verdict about whatever the live spec
said.

That cost three wrong results in one afternoon today: a manifest sed-derived
from a previous job's copy (branch had six targets, live Job had three,
reported as "all six compile"); the same class again; and a job still
pinned to OUR_REPO_REF=main while the commit under test was on a branch.

The fix in all three cases was identical — read the live object back — so
the script does that, before the job burns 30 minutes rather than after.
Manifest comes from `git show <ref>:<path>`, never the working tree.
OUR_REPO_REF is rewritten at both sites and it aborts on fewer than two (an
init/build mismatch silently runs an old build script; cost two attempts on
2026-05-17). Post-apply, live targets and ref are diffed against the ref's
manifest; mismatch deletes the Job and exits non-zero.

Ref is mandatory. Defaulting to main is precisely how you test main while
believing you tested your branch.

Verified against the live cluster, not just by reading it: injected a
manifest/live divergence and confirmed the guard named the missing target,
deleted the Job, and exited 1; confirmed the happy path verifies nine
targets; confirmed both refusal paths exit 1. The tampered-run Job was
removed and the in-flight c8c build was left untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into c8/three-unbuilt-tests ceeec4c7c9

triform-admin added 1 commit 2026-07-30 21:33:06 +00:00

docs: record the two lane traps in CLAUDE.md ... 0b0e28902a

Both cost real time today and neither is readable off the tree, which is the
bar this file sets.

1. A green build lane is green about a TARGET LIST, not about the tree.
   Three of seven test() targets were in no manifest for ~10 weeks; the first
   compile after adding them failed immediately on a class that had been
   abstract the entire time. Points at `make lint-build-targets`.

   Includes the generalizable lesson: the header's TODO had predicted the
   failure exactly, naming the build pod as what would find it. A TODO that
   names its own verification step is a defect nobody has run yet.

2. Fire the build lane with fire-build.sh, not `kubectl apply` — it reads the
   live Job object back and refuses a spec that disagrees with the ref.
   Hand-firing desynced manifest from live Job three times in one afternoon,
   each time producing a confident verdict about the wrong target list.

Also notes NINJA_KEEP_GOING, which turns one-error-per-30-minutes into one
pass that collects every failing TU.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin added 1 commit 2026-07-30 21:40:59 +00:00

fix(build): the test passthrough was missing two LINK-only overrides ... 3c5fbd86b4

cloud_browser_adm_unittests compiled clean (561/562 edges) and then failed
at link with three undefined symbols:

  ld.lld: error: undefined symbol: webrtc::metrics::HistogramAdd(...)
  >>> referenced by audio_device_buffer.cc:199 in libaudio_device_buffer.a
  ld.lld: error: undefined symbol:
          webrtc::metrics::HistogramFactoryGetEnumeration(...)
  ld.lld: error: undefined symbol: webrtc::SystemTimeNanos()
  >>> referenced by time_utils.cc:161 in libtimeutils.a

Chromium does not build libwebrtc's own versions of these. It REPLACES them
with implementations in webrtc_overrides that forward to //base:

  metrics.cc              -> //third_party/webrtc_overrides:metrics
  rtc_base/system_time.cc -> //third_party/webrtc_overrides:system_time

The cloud_browser passthrough re-exported plenty of HEADERS but neither of
these implementations, so nothing put them on a link line. They go in `deps`,
not `public_deps` — no cloud-browser TU includes them; they only have to be
present at link.

Why it never showed up before: cloud_browser_worker links the whole
content/browser stack, which drags both in transitively. Only a dep-narrow
target can see the gap, and the only dep-narrow targets in this tree are the
unit tests — one of which had been declared but built by no lane for ~10
weeks.

This is the SAME SHAPE as the //base-in-private-deps defect found earlier
today: production is fine because it links everything, the deficiency is
invisible until something links a small subset, and nothing was linking a
small subset. Two independent instances of one structural gap in one day is
the argument for `make lint-build-targets`, which now makes "declared but
built by nothing" impossible to reach.

Patch hygiene: the hunk header and the commit-message diffstat both encode
line counts, and both were updated (82 -> 117). Verified with `git apply
--check` against a clean tree — a stale count makes the patch fail to apply
at STEP 3, hours before any of this would be reached.

Not yet compiled: the re-fire is next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin commented 2026-07-30 21:41:30 +00:00

Author

Owner

Copy link

Build result: second real defect found, same structural shape

The PullRenderData fix worked — cloud_browser_adm_unittests now
compiles (561 of 562 edges done). It then failed at link:

FAILED: "./cloud_browser_adm_unittests" LINK ./cloud_browser_adm_unittests
ld.lld: error: undefined symbol: webrtc::metrics::HistogramAdd(...)
>>> referenced by audio_device_buffer.cc:199 in libaudio_device_buffer.a
ld.lld: error: undefined symbol: webrtc::metrics::HistogramFactoryGetEnumeration(...)
ld.lld: error: undefined symbol: webrtc::SystemTimeNanos()
>>> referenced by time_utils.cc:161 in libtimeutils.a

Chromium does not build libwebrtc'''s versions of these three. It replaces
them with implementations in webrtc_overrides that forward to //base
(metrics.cc, rtc_base/system_time.cc). The cloud_browser passthrough
re-exported plenty of headers but neither implementation, so nothing put them
on a link line.

Fixed in 3c5fbd8 by adding both to :webrtc_test_passthrough — in
deps, not public_deps, because no cloud-browser TU includes them; they
only need to be present at link.

Why this matters beyond the fix

This is the same structural shape as the //base-in-private-deps defect
found earlier today:

• production links the whole content/browser stack, so it drags these in
  transitively and is fine
• only a dep-narrow target can see the gap
• the only dep-narrow targets in this tree are the unit tests
• nothing was building three of those unit tests

Two independent instances of one structural gap, surfaced within hours of each
other, both by the same act of compiling code nothing had compiled. That is
the argument for make lint-build-targets: it makes "declared but built by
nothing" unreachable, so this class cannot accumulate silently again.

Patch hygiene note

patches/0003-* encodes line counts in two places — the hunk header
(@@ -0,0 +1,N @@) and the commit-message diffstat. Both had to move 82 →
117. A stale count fails at STEP 3 of the build, hours before anything here
would be reached. Verified with git apply --check against a clean tree.

Status

Re-fired as chromeless-build-c8d — using this branch'''s own
fire-build.sh, which verified the live Job spec matches 3c5fbd8 and
lists all nine targets before starting. Still NINJA_KEEP_GOING=0.

The two remaining never-compiled targets (input_dispatch,
pointer_state) have not linked yet either, so more of this class is
possible.

## Build result: second real defect found, same structural shape The `PullRenderData` fix worked — `cloud_browser_adm_unittests` now **compiles** (561 of 562 edges done). It then failed at **link**: ``` FAILED: "./cloud_browser_adm_unittests" LINK ./cloud_browser_adm_unittests ld.lld: error: undefined symbol: webrtc::metrics::HistogramAdd(...) >>> referenced by audio_device_buffer.cc:199 in libaudio_device_buffer.a ld.lld: error: undefined symbol: webrtc::metrics::HistogramFactoryGetEnumeration(...) ld.lld: error: undefined symbol: webrtc::SystemTimeNanos() >>> referenced by time_utils.cc:161 in libtimeutils.a ``` Chromium does not build libwebrtc'''s versions of these three. It **replaces** them with implementations in `webrtc_overrides` that forward to `//base` (`metrics.cc`, `rtc_base/system_time.cc`). The cloud_browser passthrough re-exported plenty of headers but neither implementation, so nothing put them on a link line. Fixed in `3c5fbd8` by adding both to `:webrtc_test_passthrough` — in `deps`, not `public_deps`, because no cloud-browser TU includes them; they only need to be present at link. ### Why this matters beyond the fix This is the **same structural shape** as the `//base`-in-private-deps defect found earlier today: - production links the whole content/browser stack, so it drags these in transitively and is fine - only a **dep-narrow** target can see the gap - the only dep-narrow targets in this tree are the unit tests - nothing was building three of those unit tests Two independent instances of one structural gap, surfaced within hours of each other, both by the same act of compiling code nothing had compiled. That is the argument for `make lint-build-targets`: it makes "declared but built by nothing" unreachable, so this class cannot accumulate silently again. ### Patch hygiene note `patches/0003-*` encodes line counts in **two** places — the hunk header (`@@ -0,0 +1,N @@`) and the commit-message diffstat. Both had to move 82 → 117. A stale count fails at STEP 3 of the build, hours before anything here would be reached. Verified with `git apply --check` against a clean tree. ### Status Re-fired as `chromeless-build-c8d` — using this branch'''s own `fire-build.sh`, which verified the live Job spec matches `3c5fbd8` and lists all nine targets before starting. Still `NINJA_KEEP_GOING=0`. The two remaining never-compiled targets (`input_dispatch`, `pointer_state`) have not linked yet either, so more of this class is possible.

triform-admin commented 2026-07-30 21:47:08 +00:00

Author

Owner

Copy link

✅ Both fixes confirmed by the build lane

chromeless-build-c8d (fired via this branch'''s own fire-build.sh,
live spec verified against 3c5fbd8):

=== all LINK lines ===
./cb_wire_envelope_unittests
./cloud_browser_adm_unittests
./cloud_browser_input_dispatch_unittests
./cloud_browser_pointer_state_unittests

=== FAILED: count ===
0

All four previously-unbuilt test binaries compile and link. That is the
first time cloud_browser_adm_unittests, cloud_browser_input_dispatch_unittests
and cloud_browser_pointer_state_unittests have ever been produced by any
lane.

Worth separating which fix did what, since it was not obvious in advance:

• input_dispatch / pointer_state linked with no new work — the
  //base → public_deps change (PR #57, merged earlier today) was already
  sufficient for them. Their long absence from the lane was very likely caused
  by that defect, exactly as suspected.
• adm_unittests needed both of today'''s fixes: PullRenderData to
  compile, and the :metrics / :system_time passthrough deps to link.

So the ten-week gap was hiding two independent defects, of two different
kinds (an abstract class and a link-time gap), in code that had been reviewed
and merged.

Build is still running (worker at 263/654). STEP 7 will execute all six test
binaries — that result will be posted here.

## ✅ Both fixes confirmed by the build lane `chromeless-build-c8d` (fired via this branch'''s own `fire-build.sh`, live spec verified against `3c5fbd8`): ``` === all LINK lines === ./cb_wire_envelope_unittests ./cloud_browser_adm_unittests ./cloud_browser_input_dispatch_unittests ./cloud_browser_pointer_state_unittests === FAILED: count === 0 ``` **All four previously-unbuilt test binaries compile and link.** That is the first time `cloud_browser_adm_unittests`, `cloud_browser_input_dispatch_unittests` and `cloud_browser_pointer_state_unittests` have ever been produced by any lane. Worth separating which fix did what, since it was not obvious in advance: - **`input_dispatch` / `pointer_state` linked with no new work** — the `//base` → `public_deps` change (PR #57, merged earlier today) was already sufficient for them. Their long absence from the lane was very likely caused by that defect, exactly as suspected. - **`adm_unittests` needed both of today'''s fixes**: `PullRenderData` to compile, and the `:metrics` / `:system_time` passthrough deps to link. So the ten-week gap was hiding **two** independent defects, of two different kinds (an abstract class and a link-time gap), in code that had been reviewed and merged. Build is still running (worker at 263/654). STEP 7 will execute all six test binaries — that result will be posted here.

triform-admin commented 2026-07-30 21:56:49 +00:00

Author

Owner

Copy link

STEP 7 ran all six binaries — 89 tests, first execution ever for four of them

STEP 6/10 autoninja OK (671s)          <- everything compiled AND linked
STEP 7/10 unit tests START
  cloud_browser_encoder_unittests             43 tests  SUCCESS
  cloud_browser_framesink_capturer_unittests  17 tests  SUCCESS
  cb_wire_envelope_unittests                  32 tests  SUCCESS
  cloud_browser_adm_unittests                 -- could not start (below)
  cloud_browser_input_dispatch_unittests       3 tests  SUCCESS
  cloud_browser_pointer_state_unittests        ? tests  SUCCESS

89 tests executed, all passing. Including C4'''s portal-dialect tests,
which now have run on real hardware rather than only under vitest:

[3/32] CbWireEnvelopeDecodeTest.AcceptsPortalFlatOffer (0 ms)
[4/32] CbWireEnvelopeDecodeTest.AcceptsPortalFlatIce (0 ms)
[5/32] CbWireEnvelopeDecodeTest.PortalFlatIceWithoutCandidateIsEndOfCandidates
[11/32] CbWireEnvelopeEncodeTest.AlwaysEmitsCanonicalNeverPortal (0 ms)

The one remaining failure is the build image, not the code

cloud_browser_adm_unittests: error while loading shared libraries:
libX11.so.6: cannot open shared object file: No such file or directory

The ADM test links the real libwebrtc Linux audio device module, which drags
in the X client libraries. The build container installs compilers and codec
headers but no runtime X libs — enough to compile everything, not enough
to run this one binary.

That gap was invisible for the same reason as everything else in this PR: for
~10 weeks STEP 7 ran only two binaries, and neither touches X.

Worth noting the failure SHAPE, because it is misleading: the binary exits
127 before main(), so STEP 7 reports unit tests failed while zero tests
have run. That reads exactly like a code defect and is not one. (Same family
as the "green by skipping" traps this branch has been chasing — here the
verdict is red for a reason unrelated to the assertions.)

Fixed by adding the runtime libs to the manifest'''s apt line. Only
libx11-6 is strictly required today; the rest of the standard Chromium
headless set is included because the next test target that links more of the
browser will need it, and each discovery costs a 12-minute build. Package
names verified against debian:bookworm-slim before firing, rather than
finding a typo 12 minutes in.

## STEP 7 ran all six binaries — 89 tests, first execution ever for four of them ``` STEP 6/10 autoninja OK (671s) <- everything compiled AND linked STEP 7/10 unit tests START cloud_browser_encoder_unittests 43 tests SUCCESS cloud_browser_framesink_capturer_unittests 17 tests SUCCESS cb_wire_envelope_unittests 32 tests SUCCESS cloud_browser_adm_unittests -- could not start (below) cloud_browser_input_dispatch_unittests 3 tests SUCCESS cloud_browser_pointer_state_unittests ? tests SUCCESS ``` **89 tests executed, all passing.** Including C4'''s portal-dialect tests, which now have run on real hardware rather than only under vitest: ``` [3/32] CbWireEnvelopeDecodeTest.AcceptsPortalFlatOffer (0 ms) [4/32] CbWireEnvelopeDecodeTest.AcceptsPortalFlatIce (0 ms) [5/32] CbWireEnvelopeDecodeTest.PortalFlatIceWithoutCandidateIsEndOfCandidates [11/32] CbWireEnvelopeEncodeTest.AlwaysEmitsCanonicalNeverPortal (0 ms) ``` ### The one remaining failure is the build image, not the code ``` cloud_browser_adm_unittests: error while loading shared libraries: libX11.so.6: cannot open shared object file: No such file or directory ``` The ADM test links the real libwebrtc Linux audio device module, which drags in the X client libraries. The build container installs compilers and codec headers but no runtime X libs — enough to **compile** everything, not enough to **run** this one binary. That gap was invisible for the same reason as everything else in this PR: for ~10 weeks STEP 7 ran only two binaries, and neither touches X. Worth noting the failure SHAPE, because it is misleading: the binary exits 127 before `main()`, so STEP 7 reports `unit tests failed` while zero tests have run. That reads exactly like a code defect and is not one. (Same family as the "green by skipping" traps this branch has been chasing — here the verdict is red for a reason unrelated to the assertions.) Fixed by adding the runtime libs to the manifest'''s apt line. Only `libx11-6` is strictly required today; the rest of the standard Chromium headless set is included because the next test target that links more of the browser will need it, and each discovery costs a 12-minute build. Package names verified against `debian:bookworm-slim` before firing, rather than finding a typo 12 minutes in.

triform-admin added 1 commit 2026-07-30 21:57:10 +00:00

fix(build): the build image could compile the ADM test but not run it ... 1197256b12

STEP 6 finished clean (671s, every target compiled AND linked). STEP 7 then
ran all six binaries — 89 tests, all passing — except one, which never
started:

  cloud_browser_adm_unittests: error while loading shared libraries:
  libX11.so.6: cannot open shared object file: No such file or directory

The ADM test links the real libwebrtc Linux audio device module, which pulls
in the X client libraries. The build container installs compilers, codec
headers and build tools — enough to COMPILE everything, not enough to RUN
this one binary.

Invisible for the same reason as everything else in this branch: for ~10
weeks STEP 7 ran exactly two binaries and neither touches X.

The failure SHAPE is the part worth remembering. The binary exits 127 before
main(), so STEP 7 reports "unit tests failed" having run zero tests. That
reads like a code defect and is not one — the same family of misleading
verdict as the green-by-skipping traps, inverted: red for a reason that has
nothing to do with the assertions.

Only libx11-6 is strictly required today. The rest of the standard Chromium
headless runtime set is included deliberately: the next test target that
links more of the browser will need it, and each discovery costs a 12-minute
build to find. All 22 package names verified to resolve on
debian:bookworm-slim before firing — a typo here also costs 12 minutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin added 1 commit 2026-07-30 22:09:11 +00:00

ci: retrigger — smoke test timed out on a CDP call, not a regression ... 4df2e8b897

The Container smoke test failed with:

  [smoke] DevTools is up after 3s
  [smoke] page target: ws://127.0.0.1:9222/devtools/page/86C6490...
  TimeoutError: timed out

The container booted, supervisord brought up xvfb/pulseaudio/chromium/
devtools-proxy, and DevTools answered in 3s. The failure is the CDP command
after attach.

This PR cannot cause it. Its entire diff is C++ test sources, build
manifests, host-side lints and docs — nothing that ships in the worker
image, and the smoke test runs against a prebuilt image
(cr7727-f9e46a1434b5) that this branch does not rebuild.

Checked rather than assumed: the same job on the same image passed on main
at the C7 merge (job 220770, status=success). So this is a flake in the CDP
attach path, not a regression.

Empty commit because Forgejo v14 has no rerun API.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin added 1 commit 2026-07-30 22:16:21 +00:00

fix(build): give the ADM test a PulseAudio server so its verdict discriminates ... 4a4f85154d

With libX11 present, cloud_browser_adm_unittests finally started — and
immediately hit the next layer of the same gap:

  Can't load libpulse.so.0 : cannot open shared object file
  failed to initialize PulseAudio
  M55-R2-VERDICT: {"verdict":"FAIL","reason":"CreateCloudBrowserNative
    AudioDeviceModule returned nullptr — likely patches/0003
    audio_device_impl widening needed (see R1 TODO)"}

That reason string is wrong in this environment, and fixing that is the
whole point of this commit.

cb_audio_device_module_test.cc is a deliberate RED-first test whose file
bottom carries an escalation table keyed on WHICH failure it observes:

  ADM nullptr           -> widen patches/0003 first
  RecordingDevices()==0 -> escalate to choice (a), a libpulse dynamic loader
  no samples / low RMS  -> escalate to (a)

Without a libpulse client library the test can only ever report the first
row, so the table cannot discriminate and the RED is unusable as evidence in
either direction. Worse, it actively points at the wrong fix: someone acting
on that verdict would go widen patches/0003 to solve a missing .so.

The test needs a reachable SERVER, not just the library — it drives the real
libwebrtc Pulse backend — so the daemon is started too. Placement is
load-bearing and was verified in a throwaway pod rather than guessed:

  as root:    "This program is not intended to be run as root
               (unless --system is specified)"       -> refuses
  as builder: Server String: /tmp/pulse-.../native
              Server Protocol Version: 35            -> live

Hence the start lives INSIDE the `su builder` block, not in the root
preamble above it. Same uid matters twice: the daemon refuses root, and the
client locates the server through that user's runtime dir.

Best-effort by design (`|| echo WARN`): a missing audio daemon must never
fail a compile lane. If it doesn't come up the ADM test still runs and still
reports RED, just with the pre-existing ambiguity.

To be explicit about what this does NOT do: the ADM test will still be RED.
It is designed to be until the M5.5 audio work lands. This makes its RED mean
what the escalation table says it means.

All three package names verified against debian:bookworm-slim first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin commented 2026-07-30 22:43:13 +00:00

Author

Owner

Copy link

The ADM verdict now discriminates — and it moved

With libpulse0 + a builder-owned PulseAudio daemon present, the M5.5 test
reports something different, which was the entire point of that commit:

[1/3] CloudBrowserAdmTest.SmokeAdmConstructible (17 ms)          <- PASSES now
[ RUN ] CloudBrowserAdmTest.RecordingPumpRunsOnDefaultSource
cb_audio_device_module_test.cc:262: Failure
Expected equality of these values:
  adm->InitRecording()
    Which is: -1
  0

Before: ADM construction returned nullptr → escalation table row 1 → "widen
patches/0003 audio_device_impl".

Now: the ADM constructs, Init() succeeds, RegisterAudioCallback()
succeeds, and InitRecording() returns -1. That is a different row entirely.
The old verdict was pointing at the wrong fix — someone acting on it would have
gone patching gn deps to solve a missing .so.

MonitorSourceDeliversToneRms never ran (gtest stops the fixture at the first
ASSERT_), so the RMS check — the one the spec calls load-bearing — is still
unmeasured.

Status of this test: RED by design, and that is correct

cb_audio_device_module_test.cc is a deliberate RED-first test with an
escalation table in its file bottom. It is supposed to fail until the M5.5
audio work lands. Nothing in this PR claims to make it green.

What changed is that its RED is now usable as evidence: it names a real
next step (why does the Pulse backend reject InitRecording() on the
server-default source — no source in the build container? permissions? the
choice-(a) dynamic-loader escalation?) instead of a fabricated one.

Everything else passes

cloud_browser_encoder_unittests             43 tests  SUCCESS
cb_wire_envelope_unittests                  32 tests  SUCCESS
cloud_browser_framesink_capturer_unittests  17 tests  SUCCESS
cloud_browser_input_dispatch_unittests       3 tests  SUCCESS
cloud_browser_pointer_state_unittests       10 tests  SUCCESS
cloud_browser_adm_unittests                  1 of 3 pass, RED by design

105 tests across six binaries, four of which no lane had ever built.

Open question for review

STEP 7 is fatal-on-failure (deliberately — that was PR #56's point). With a
RED-by-design test now in the lane, the lane cannot go green until M5.5 lands.
Options, in my order of preference:

1. Move the ADM test out of the compile lane into a target that runs where
   a real audio server exists (the runtime image already runs PulseAudio under
   supervisord). It is an integration test wearing a unit test's clothes.
2. Keep it in the lane and let the lane stay red until M5.5 — honest, but a
   permanently-red lane is how the last ten-week gap survived.
3. CHROMELESS_TESTS_NONFATAL=1 — rejected. That is exactly the "temporarily
   non-blocking became permanent" failure PR #56 removed.

I have not picked one; it changes what the lane means, so it wants a decision
rather than a default.

## The ADM verdict now discriminates — and it moved With `libpulse0` + a `builder`-owned PulseAudio daemon present, the M5.5 test reports something *different*, which was the entire point of that commit: ``` [1/3] CloudBrowserAdmTest.SmokeAdmConstructible (17 ms) <- PASSES now [ RUN ] CloudBrowserAdmTest.RecordingPumpRunsOnDefaultSource cb_audio_device_module_test.cc:262: Failure Expected equality of these values: adm->InitRecording() Which is: -1 0 ``` Before: `ADM construction returned nullptr` → escalation table row 1 → *"widen `patches/0003` audio_device_impl"*. Now: the ADM **constructs**, `Init()` succeeds, `RegisterAudioCallback()` succeeds, and `InitRecording()` returns -1. That is a different row entirely. The old verdict was pointing at the wrong fix — someone acting on it would have gone patching gn deps to solve a missing `.so`. `MonitorSourceDeliversToneRms` never ran (gtest stops the fixture at the first `ASSERT_`), so the RMS check — the one the spec calls load-bearing — is still unmeasured. ### Status of this test: RED by design, and that is correct `cb_audio_device_module_test.cc` is a deliberate RED-first test with an escalation table in its file bottom. It is *supposed* to fail until the M5.5 audio work lands. Nothing in this PR claims to make it green. What changed is that its RED is now **usable as evidence**: it names a real next step (why does the Pulse backend reject `InitRecording()` on the server-default source — no source in the build container? permissions? the choice-(a) dynamic-loader escalation?) instead of a fabricated one. ### Everything else passes ``` cloud_browser_encoder_unittests 43 tests SUCCESS cb_wire_envelope_unittests 32 tests SUCCESS cloud_browser_framesink_capturer_unittests 17 tests SUCCESS cloud_browser_input_dispatch_unittests 3 tests SUCCESS cloud_browser_pointer_state_unittests 10 tests SUCCESS cloud_browser_adm_unittests 1 of 3 pass, RED by design ``` 105 tests across six binaries, four of which no lane had ever built. ### Open question for review STEP 7 is fatal-on-failure (deliberately — that was PR #56's point). With a RED-by-design test now in the lane, the lane cannot go green until M5.5 lands. Options, in my order of preference: 1. **Move the ADM test out of the compile lane** into a target that runs where a real audio server exists (the runtime image already runs PulseAudio under supervisord). It is an integration test wearing a unit test's clothes. 2. Keep it in the lane and let the lane stay red until M5.5 — honest, but a permanently-red lane is how the *last* ten-week gap survived. 3. `CHROMELESS_TESTS_NONFATAL=1` — rejected. That is exactly the "temporarily non-blocking became permanent" failure PR #56 removed. I have not picked one; it changes what the lane means, so it wants a decision rather than a default.

triform-admin added 2 commits 2026-07-30 22:59:48 +00:00

Merge remote-tracking branch 'origin/main' into c8/three-unbuilt-tests dd700325e2

build: the new lint caught a live regression within hours ... 63431051ae

Merged main (which now carries C3, PR #55) into this branch and
`make lint-build-targets` immediately failed:

  build-targets-lint: test targets that NO build lane compiles:
    cb_env_config_unittests
        declared in capture/config/BUILD.gn

C3 added a test() target and put it in no build manifest — the exact defect
this branch exists to prevent, reintroduced within hours of the guard being
written, by a PR I reviewed and merged myself.

That is the whole argument for the lint, made better by an accident than any
example I could have constructed. The previous instance of this took ten
weeks to surface and did so only because someone went looking. This one took
one `make verify`.

Fixed by adding the target to the x264-t7 lane (7 of 8 now; pcf stays
t2-only by design). The lane inventory comment is updated too — it lists
every declared test target and would otherwise have drifted from the truth
it documents, which is its own small version of the same failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin commented 2026-07-30 23:00:02 +00:00

Author

Owner

Copy link

The lint caught a live regression, within hours, from a PR I merged myself

Merging current main into this branch — routine, to keep the green CI
meaningful — produced this on the first make verify:

build-targets-lint: test targets that NO build lane compiles:
  cb_env_config_unittests
      declared in capture/config/BUILD.gn

C3 (#55, merged earlier today) added a test() target and put it in no build
manifest. That is precisely the defect this PR exists to prevent, reintroduced
within hours of the guard being written, by a change I reviewed and merged.

The previous instance of this class took ten weeks to surface and only did
so because someone went looking. This one took one make verify, and it is a
better argument for the lint than any example I could have constructed
deliberately.

Fixed by adding the target to the x264-t7 lane — now 7 of 8 (pcf_unittests
stays t2-only by design, which the lint deliberately permits). The lane's
inventory comment is updated too; it enumerates every declared test target and
would otherwise drift from the truth it documents, which is a small version of
the same failure.

Coverage now:

build-job-x264-t7.yaml   7/8  cb_env_config cb_wire_envelope cb_adm
                              cb_encoder cb_framesink_capturer
                              cb_input_dispatch cb_pointer_state
build-job-x264-t2.yaml   3/8  + cb_pcf (t2-only, deliberate)
others                   2/8

Branch is now current with main and re-running CI.

## The lint caught a live regression, within hours, from a PR I merged myself Merging current `main` into this branch — routine, to keep the green CI meaningful — produced this on the first `make verify`: ``` build-targets-lint: test targets that NO build lane compiles: cb_env_config_unittests declared in capture/config/BUILD.gn ``` C3 (#55, merged earlier today) added a `test()` target and put it in no build manifest. That is precisely the defect this PR exists to prevent, reintroduced within hours of the guard being written, by a change I reviewed and merged. The previous instance of this class took **ten weeks** to surface and only did so because someone went looking. This one took one `make verify`, and it is a better argument for the lint than any example I could have constructed deliberately. Fixed by adding the target to the x264-t7 lane — now 7 of 8 (`pcf_unittests` stays t2-only by design, which the lint deliberately permits). The lane's inventory comment is updated too; it enumerates every declared test target and would otherwise drift from the truth it documents, which is a small version of the same failure. Coverage now: ``` build-job-x264-t7.yaml 7/8 cb_env_config cb_wire_envelope cb_adm cb_encoder cb_framesink_capturer cb_input_dispatch cb_pointer_state build-job-x264-t2.yaml 3/8 + cb_pcf (t2-only, deliberate) others 2/8 ``` Branch is now current with main and re-running CI.

triform-admin added 1 commit 2026-07-31 08:41:21 +00:00

docs: decisions the user must make go through AskUserQuestion, always ... 38d362c2ba

Adds a "Working with the user" section to CLAUDE.md, above Conventions so it
is read before style guidance.

The rule: if the user has to decide something, ASK — via the tool, every
time. Not a bullet list, not a closing paragraph.

Why it is worth a section rather than a line. A message that ends "three
things are yours to decide" reads as FINISHED. The user sees a completed task
and no prompt, so the thread goes dead while the agent believes it is
politely waiting. I did exactly this across several consecutive messages on
2026-07-30 — three open decisions (merge a green PR, where a RED-by-design
test should live, whether to reshard a backup CronJob) sat in prose and the
session stalled every time. From the other side that is indistinguishable
from "the agent is done and has stopped."

The section also says what NOT to ask about — conventional defaults, facts
readable from the repo, decisions already made — because an agent that asks
about everything is its own kind of noise, and that failure mode is the
reason the habit of "just write it in the summary" forms.

Same shape as the rest of this file: it records a specific failure and what
it cost, not an abstract preference.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

triform-admin merged commit 407a690a35 into main 2026-07-31 08:44:45 +00:00

triform-admin referenced this pull request from a commit 2026-07-31 08:44:46 +00:00

Merge pull request 'build: the lane was green about files it never compiled — fix + two guards' (#60) from c8/three-unbuilt-tests into main

triform-admin referenced this pull request 2026-07-31 09:01:50 +00:00

build: the ADM test is built here but runs where audio exists #61

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

triform/chromeless!60

No description provided.