fix(release): retain source ancestry after the batch B squash #111

Merged

triform-admin merged 25 commits from agent/mapdb-provenance into main 2026-09-09 22:53:49 +00:00

Conversation 0 Commits 25 Files changed -

triform-admin commented 2026-09-09 22:48:57 +00:00

Owner

Copy link

PR #109 integrated batch B via a squash, leaving the recorded guest build source f9deaaa7ef outside main ancestry. The downstream provenance guard consequently rejects the published release.

This merge restores that source ancestry and preserves the exact current main tree, including newer upload diagnostics, documentation and release pins. There are zero file changes and no new runtime artifact. Validation: git diff HEAD^1 HEAD is empty; the recorded source is an ancestor of this head; repository verification is running. Merge this PR with a merge commit, preserving its parents, to retain the provenance being repaired.

PR #109 integrated batch B via a squash, leaving the recorded guest build source f9deaaa7efc66891ca3a5b5303cf8c67198981ed outside main ancestry. The downstream provenance guard consequently rejects the published release. This merge restores that source ancestry and preserves the exact current main tree, including newer upload diagnostics, documentation and release pins. There are zero file changes and no new runtime artifact. Validation: git diff HEAD^1 HEAD is empty; the recorded source is an ancestor of this head; repository verification is running. Merge this PR with a merge commit, preserving its parents, to retain the provenance being repaired.

triform-admin added 25 commits 2026-09-09 22:48:58 +00:00

feat(upload): <input type=file> works — both halves were missing ... ad843ac38c

A file input on a streamed page did nothing at all, and it took two
independent fixes to explain why:

  1. CbWebContentsDelegate had no RunFileChooser override. Chromium's
     default auto-cancels, so the page was told "no file selected"
     before anything could ask the viewer for a byte.
  2. The "files" DataChannel was bound to CbFileUploadRelay, which
     forwarded frames to a WebSocket bridge (capture/file-bridge/)
     over a client constructed with url="off" — permanently
     disabled(), dropping every frame. Same shape as the clipboard
     relay's four-month outage, and the log line even said so.

Either fix alone changes nothing, which is presumably why neither
happened. Both land here.

CbFileUploadReceiver terminates docs/protocols/file-upload.md v1 in
the browser process: chunks are base64-decoded onto a MayBlock
sequence into <profile>/Uploads, the finished file is read BACK off
disk to verify the client's sha256 (what is verified is what the page
will get), and the result resolves the content::FileSelectListener
that RunFileChooser parked. The sidecar is deployed by nothing today,
so nothing regresses; its 35 KB of unreferenced source is deleted.

Read from the pinned tree rather than remembered, and two drafts were
wrong until it was:

  * NativeFileInfo::display_name is what the page reads as File.name.
    Left empty, blink falls back to the base of file_path — our
    sanitised "<id>__<name>" — so a site echoing the filename or
    checking its extension would see the wrong thing. The raw name is
    now carried alongside the on-disk one.
  * FileSelected's base_dir is empty except for kUploadFolder
    (file_select_listener.h:24). We reject that mode, so it is always
    empty; passing uploads_dir_ advertised an enumeration root that
    described nothing.

Two more defects found by re-reading, not by the compiler:

  * Upload::received was never incremented, so progress always
    reported 0 and the cumulative size guard tested 0 > declared_size
    — i.e. never fired. It now counts DECODED bytes; an earlier draft
    passed the base64 length through, which overstates by a third.
  * Every abandon path left partial bytes on disk. Since the session
    quota only credits uploads that SUCCEED, a client retrying a bad
    hash could fill the guest's disk without ever tripping it. All
    six paths now go through AbandonUpload (erase + delete + reply).

The listener contract (resolve exactly once, or chromium CHECKs on
release) is held from three directions, because no one of them is
enough: the receiver runs its own 5-minute deadline (the control
request's is cancelled the moment the viewer answers, so a viewer who
says "sending" and then vanishes would strand it); re-arm and teardown
cancel before destroying; and the client answers every branch,
including when its own picker throws.

Client: control.ts learns kind=file_chooser and main.ts shows a picker
bar, since a file dialog needs a user gesture and this request arrives
from the network. Five new tests, two mutation-checked.

Docs: the protocol spec's server half was describing the sidecar. Its
MIME allowlist and virus scan do not exist here and are now marked NOT
IMPLEMENTED rather than left as claims; target_selector is ignored;
attached_via can no longer be domSetFileInputFiles; the error-code
table is replaced with the codes actually emitted.

UNVERIFIED: capture/ does not compile locally. The build lane has not
run on this commit.

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

build: the t7 memory request said 64Gi and meant 72Gi — duplicate YAML key ... 2be9512f5c

build-job-x264-t7.yaml carried this, and had since 2026-08-21:

    memory: "64Gi"
    memory: "72Gi"

Twenty lines of comment above the first argue — with measurements from
two OOM evictions — for 64Gi. YAML's last-key-wins gave every build
72Gi. The 64Gi edit had been inserted ABOVE the existing line instead
of replacing it.

Nothing surfaced it in three weeks. kubectl apply accepts a duplicate
mapping key silently, PyYAML accepts it silently, both values schedule
on triform-7, so every build worked and the manifest simply did not
mean what it said. No other manifest in the repo has one — the other
five build jobs have exactly two `memory:` keys, which is requests +
limits.

The class matters more than the instance. This repo's manifests are
heavily commented precisely BECAUSE the numbers are load-bearing and
hard-won (CLAUDE.md's OutOfcpu/OutOfmemory traps are about these exact
lines), and a silently discarded value makes every one of those
comments untrustworthy. Same shape as "a guard whose skipped path is
silent is not a guard": the discarded value reports identically to the
applied one.

So: lint-yaml-dupe-keys, wired into `make lint`. Checked against the
whole tree before landing (CLAUDE.md's rule) — zero findings across 59
files once t7 is fixed, and the 14 Go-templated Helm files are skipped
BY NAME with the skip reported, so the lint can never quietly check
nothing. Negative control run both ways: reintroducing the exact
defect fails it at the right line; removing it passes.

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

test(interactive): a file input on the remote page, satisfied by the viewer ... a6fcb928d6

Nine checks in a new `uploads` suite. Its oracle is deliberately the
REMOTE PAGE's own <input>, not the guest's Uploads/ directory: bytes on
disk prove the transport ran, while the defect this exists for is that
the page's `change` event never fired at all. A suite that checked the
filesystem would have passed against a guest where <input type=file>
was still completely dead.

The preflight reports the `files` and `control` channels SEPARATELY
because the two halves failed independently — no RunFileChooser
override (chromium auto-cancels, page told "no file selected") and a
files channel bound to a relay with no backend. Either one alone
presents as "the file input does nothing", and naming the wrong half
sends the next person to the wrong file.

Also asserts File.name is what the viewer picked. An empty
NativeFileInfo::display_name makes blink fall back to the base of the
on-disk path — our sanitised "<upload_id>__<name>" — and every other
check in the suite would pass right through that. Contents are read
back by the page too, since a correct name and size with wrong bytes
is exactly what a base64 or chunk-ordering bug looks like.

Driving the client's own picker needs DOM.setFileInputFiles: a real
file dialog cannot be scripted, and the click that OPENS the chooser
still goes through the input channel as a genuine mouse event, which
is what makes chromium call RunFileChooser in the first place.

Not yet run — it needs a guest image containing the receiver, and that
build is in the lane now.

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

feat(client): audio you can actually hear, and a test that would notice ... 70e3b14558

The guest has sent an audio track since the first session. `client/
index.html` carried `<video autoplay playsinline muted>` with no
control anywhere, so every byte arrived, decoded, and was discarded.
The product had audio in the sense that the SDP mentioned it.

`muted` on the element is NOT the bug and is kept: an unmuted autoplay
is refused by every browser's autoplay policy, and the failure is not
"no sound" but a rejected play() that stalls the entire video. What
was missing is a way OUT of that state, which the policy is explicit
about — a user gesture. So: an Unmute button, which is that gesture,
and which retries play() inside the click rather than on a later tick
where the gesture no longer counts. If the retry is still refused the
button re-mutes itself rather than claiming sound the user cannot
hear.

Mute state is read off the <video> rather than tracked in a variable,
and a `volumechange` listener keeps the label honest when the browser's
own media controls change it. The state deliberately SURVIVES a
reconnect — only the control is disabled between sessions — because a
broker blip silently re-muting someone who unmuted is its own bug.

Three checks added to the interactive stats suite, splitting two facts
that fail independently: inbound-rtp audio bytesReceived > 0 is the
transport (ADM, encoder, transceiver), and the element being unmuted
after a real click is whether a human hears anything. A stack that
gets the first right and the second wrong is silent — which is exactly
what shipped, and what no existing check could have caught. The
suite's one previous mention of "audio" asserts a SENDER kind during
camera passthrough, i.e. the opposite direction.

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

feat(client): fullscreen, and the guest event that nobody was listening to ... d86680e4be

Two halves of one gap.

The button: fullscreen the STAGE rather than the document, so the
stream fills the window and keeps its aspect ratio while the panel and
log get out of the way. The label follows `document.fullscreenElement`
rather than a flag of our own, because Escape and the browser's own
control both leave fullscreen without going through the button. Each
transition re-syncs the viewport, since the stage just changed size.

The event: `CbWebContentsDelegate` has emitted `fullscreen_changed` on
the control channel since it was written, and control.ts logged it and
dropped it — the one comment on that branch even said "fullscreen_
changed today; nothing to answer", which was true and beside the
point. There was nothing to ANSWER and something to DO. A streamed
page calling requestFullscreen (a video player, a game) gets it
granted by chromium, its :fullscreen CSS applies and its layout
changes, while the viewer still sees a 380px sidebar.

attachControlChannel gains an onEvent seam, guarded: a throwing
handler must not kill the channel, because every later frame arrives
through the same callback — including the ui_requests the guest
BLOCKS on. That guard is the mutation-checked test.

Following the event can be refused, since the browser wants a user
gesture and a network event is not one. The refusal is logged rather
than swallowed, and the page ends up no worse off than before.

Three new tests, 295 passing.

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

feat(client): keys typed into the client no longer stream to the guest ... cf03a61322

Keys are listened for on the WINDOW, and they have to be — a <video>
cannot hold focus in a way that delivers key events, so a
target-scoped listener would receive nothing. The consequence was that
every key in the page went to the cloud browser: the URL a user typed
into the address bar was streamed keystroke by keystroke into whatever
the remote page had focused, and Cmd/Ctrl+L, +T, +W and +R fired in
BOTH browsers at once.

`keyBelongsToClient` decides by WHERE FOCUS IS, not by which key it
is. Anything the viewer can type into or activate (input, textarea,
select, button, contenteditable) and the two client-owned overlays
(the dialog prompt, the file picker) keep their keys; everything else
goes to the guest.

Deliberately NOT an enumeration of "browser shortcuts". Intercepting
Cmd+W means preventDefault on a key the viewer may genuinely want
locally, and a stream that swallows it is worse than one that merely
does not forward it. Duplicating it was the bug.

The gate applies to keyup as well, which carries a real risk: focus
moving between a key's down and its up would leave the guest thinking
the key is held. The alternative leaks every release of every key
typed into the client's own UI, and the guest's dispatcher tracks held
modifiers from exactly these events. The policy is stable for the life
of a keypress, so the risk does not materialise — written down at the
call site rather than discovered later.

The rule lives in src/input.ts as a pure function over a duck-typed
target, because this package's tests run in node with no DOM and a
policy nobody can test is how the previous one (there wasn't one)
survived. `attach()` takes it as an option defaulting to "forward
everything", so every existing caller is unchanged.

Ten tests. Mutation-checked at all three points: the rule, the keydown
gate, the keyup gate, and the permissive default. Worth recording that
the first keydown mutation SILENTLY FAILED TO MATCH and the arm
reported 47/47 green — a clean file compared against itself, exactly
the trap CLAUDE.md describes. Re-run with the anchor count asserted, it
fails as it should.

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

fix(client): every paste was sent twice and took effect once ... 998e77390b

`InputChannel.attach` wired the browser's `paste` event to a
`clipboard_paste` envelope on the INPUT channel, while `main.ts`
separately sent `clipboard_offer` on the CLIPBOARD channel. Two
envelopes per paste, one of which does nothing.

Nothing surfaced it, and the reason is worth writing down: the guest
lists `clipboard_paste` in `kKnownInputTypes`, so the dispatcher
accepts it and does NOT report it through OnInputEventUnknownType. It
is dropped silently by a lookup table that says it is known.
`cb_input_dispatch_clipboard.h` documents the gap on purpose ("we
leave the dispatch surface unclaimed here") and the M4 rank that was
to claim it never landed.

Removed the listener, not the sender: `sendClipboardPaste` stays for
anyone driving the channel directly, now carrying a comment saying
where it goes (nowhere). `clipboard_copy_request` stays wired — that
one IS consumed, arming the guest's copy window
(cb_input_dispatch_composite.cc:77), so removing the copy listener
would have broken a working feature while chasing a dead one.

The spec claimed "the server writes this into the remote clipboard and
synthesizes a paste action". It does not, and has never. Marked as
specified-but-unimplemented with a pointer to `clipboard_offer`, which
is what works. The envelope itself stays in v1 — removing a type is a
wire break, and a future implementation may still claim it.

Two tests, mutation-checked by putting the listener back.

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

fix: ReadDict needs its options argument — the lane found it, precedent would have ... 57bdcfc65e

The build lane failed on exactly one thing across 638 compile steps:

  cb_file_upload_receiver.cc:183:74: error: too few arguments to
  function call, expected at least 2, have 1
      base::JSONReader::ReadDict(json)

`options` has no default at 7727. Every other ReadDict caller in this
tree already passes base::JSON_PARSE_RFC — cb_control_channel.cc:257,
cb_clipboard_relay.cc:109, cb_devtools_agent.cc:664 and :750. This is
the check CLAUDE.md prescribes ("grep for existing uses of it in
capture/ before using a Chromium API"), and I ran it for AppendToFile,
crypto:#️⃣:Sha256, GetDeleteFileCallback, FileSelected and the
chooser mojom, and skipped it for this one line.

Everything else in batch B compiled: cb_web_contents_delegate.o and
cloud_browser_browser_main_parts.o both built clean, so the
RunFileChooser override, the receiver wiring, the re-arm/teardown
cancellation and the new GN target are all confirmed by the compiler.
Re-verified the remaining APIs against the pinned tree afterwards
(JSONWriter::Write, ThreadPool::CreateSequencedTaskRunner,
OneShotTimer::Start, PostTaskAndReplyWithResult, base::DictValue).

Also in this commit: TokenRefresher finally has a caller.

It has existed, implemented and unit-tested, with none. Tokens carry a
5-15 minute TTL while the gateway's login cookie lasts 12 hours, and
ReconnectingWebSocket froze the signaling URL — token and all — at
construction. So any session that outlived one TTL and then had to
redial presented an EXPIRED credential; the broker rejects that at the
handshake, which is indistinguishable from "the broker is down", and
it gets more likely the longer a session runs. Precisely when
reconnecting matters.

The socket now takes a URL SUPPLIER (a plain string still works, so
every existing caller is unchanged) resolved per dial, and the session
adopts the token it already fetched rather than issuing a second one.
TokenRefresher gained adopt() + scheduleNext() for that. It is stopped
on teardown: a live refresher after disconnect keeps minting
credentials every 15 minutes for a session nobody holds.

Four tests. The redial one is mutation-checked by re-freezing the URL
at construction, which reproduces the original bug exactly.

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

chore(cv2-build): record guest release cr7727-e0e029b5c6ac (batch B) and pin the stack ... 643c547ba1

Built from e0e029b, which is the commit the lane verified: 639 steps,
0 failed, cloud_browser_worker and all eight test targets linked.

Digest sha256:e6589bd4. Carries batch R (merged as 30178cd) plus
CbFileUploadReceiver and the RunFileChooser override, so `<input
type=file>` reaches the viewer for the first time.

Both files in one commit, per the push script's own warning: recording
only the json is what made main drift before, and `make
lint-deploy-pin` exists to catch exactly that.

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

fix(upload): AppendToFile does not create the file — every upload died on chunk 0 ... 31afc97854

Measured on the live guest at cr7727-e0e029b5c6ac. The whole path
worked: the chooser was parked, the request crossed the control
channel, the viewer saw a picker, the bytes arrived on the files
channel — and then the FIRST chunk failed with `write_failed` and the
chooser was cancelled, so the page saw "no file selected".

base::AppendToFile opens with `O_WRONLY | O_APPEND` and no `O_CREAT`
(base/files/file_util_posix.cc:1269), so it cannot create the file it
appends to. Its header says only "Appends |data| to |filename|" —
true, and it reads as if a missing file is created. `CreateDirectory`
on the parent was already there; the FILE was the gap.

The first chunk now goes through base::WriteFile, which "gets created
with read/write permissions for all" if absent (file_util.h:577-579)
and truncates if present. Truncating matters: a leftover file at that
path can only be a stale partial from an abandoned upload_id, and
appending to it would silently corrupt the new upload.

This is the third API in this file whose exact contract mattered, and
the second the header alone would not have told me — the lane caught
ReadDict's required options argument, and only running it against a
real guest caught this one. A unit test would not have: it is a
property of the filesystem call, not of my logic.

Also in this commit, two harness fixes found the same way:

- The upload fixture's `<input type=file>` was a ~200x20 control, and
  the click lands at its CENTRE — on the "No file chosen" label, not
  the button. window.__clicks stayed 0 and the suite reported "no
  picker appeared", which reads as a broken RunFileChooser override
  and is not one. The download fixture's anchor is a padded block for
  exactly this reason and says so in a comment; I did not copy it.
- Added the click-landed check itself, so "the mouse missed" can never
  again be reported as "the upload is broken". Same oracle the
  download suite has carried since it learned the lesson.

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

docs: what a green lane does not tell you, and the batch B pins ... c4703df6b5

Records the two API traps from batch B where a correct signature was
not a correct call, plus the four that were simply right.

The load-bearing one is base::AppendToFile: it compiled, linted, and
passed the lane, then failed on every upload's first chunk because its
POSIX implementation opens O_WRONLY|O_APPEND with no O_CREAT. The
header says "Appends |data| to |filename|" — true, and it reads as if
a missing file is created.

The general form, now in CLAUDE.md next to the lint-cxx caveat: a lint
clean means one mistake is absent, a green lane means it builds, and
neither means it works. When a contract is about a SIDE EFFECT — does
it create, truncate, require the parent — the declaration will not say.
Read the _posix.cc implementation or budget a live run.

Also pinned: FileSelected's base_dir is empty except for kUploadFolder,
and NativeFileInfo::display_name is what the page reads as File.name
(empty falls back to the on-disk basename, so a sanitised name leaks
into the DOM while every other check passes).

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

fix(upload): every chunk-drop path was silent — make them all say why ... eee0fae6f8

Chasing why uploads failed cost two rebuild cycles, and the reason is
that the receiver could lose a chunk without saying so. The visible
symptom was `file_upload_end` replying `write_failed: could not read
the file back` — a truthful description of a consequence, four steps
from any cause.

Measured, not guessed: hooking RTCDataChannel.send in the client
proved it sends start/chunk/end in order, and the guest's error lands
16ms later. So the chunk reaches the wire and vanishes inside the
receiver, in one of two early returns — a missing-field check with a
bare `return`, and an unknown-upload check behind VLOG(1), i.e.
invisible in production.

Both now LOG(WARNING) with the specific field or id. `file_upload_end`
additionally says when NOT ONE chunk was accepted, because that is the
cause of the read-back failure and it is known at that point. And
HandleEnvelope logs every inbound type with its size, which is the one
line that would have answered "did a chunk arrive?" immediately.

This is CLAUDE.md's own rule — "a guard whose skipped path is silent
is not a guard" — applied to code I wrote two days after re-reading it.
The drop reported identically to success.

Also corrects a comment I had just written asserting `!seq` would
reject seq==0. It would not: std::optional::operator bool IS
has_value(), and cb_input_dispatch_mouse.cc relies on exactly that for
x/y coordinates that are legitimately 0. `.has_value()` stays spelled
out for clarity, since the three operands in that condition convert to
bool for two different reasons, but it is not a fix and the comment no
longer claims it is.

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

feat(client): a HUD, because the stats event had no subscriber ... 271ccacfd2

ChromelessSession has emitted a full StatsSample once a second since
T82. The client assembled it, shipped it to the broker over the stats
data channel, and dropped it on the floor locally — so a viewer
watching a stream degrade had no way to tell a slow network from a
broken guest. The panel said `connected` either way, which is the
least useful thing it could say at exactly the moment someone needs to
know.

Five numbers and a dot: fps, bitrate, RTT, loss, resolution, quality.

src/hud.ts is deliberately PURE — two samples in, display strings out,
no DOM and no clock — because this package's tests run in node and an
untestable policy is how the last one (there wasn't one) survived.

The deltas are the whole difficulty. Every counter in a StatsSample is
CUMULATIVE, so a HUD that renders them raw shows numbers that only
climb and mean nothing; each rate is computed against the previous
sample's timestamp. Three specific decisions, each with a test:

- No previous sample reads "—", not 0. "0 fps" and "I don't know yet"
  are different claims and the first one looks like a defect.
- Loss is over the INTERVAL, not cumulative. A session that lost 200
  packets in its first second and none since is healthy now; a
  cumulative ratio would keep accusing it for as long as it runs.
- Two samples sharing a timestamp render "—", not Infinity fps, which
  would read as a spectacular success.

Resolution comes off the <video>'s videoWidth/Height rather than
getStats: that is what is actually being PAINTED, and it is the number
a user can check against their own window.

18 tests, mutation-checked at the two places that matter (dropping the
byte delta, dropping the interval guard). Four interactive checks read
what is ON SCREEN rather than what getStats returns, since the point is
that the numbers reach a person.

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

docs(findings): the upload chunk reaches the guest and vanishes ... b47f108f90

Records what is ESTABLISHED rather than what is suspected, because
this defect has already survived two theories.

Established by measurement: the client sends start/chunk/end (hooking
RTCDataChannel.send in the viewer's browser), start and end are both
processed by the guest (it logs the name, size and destination, and
the error arrives 16ms later from the end handler), and Uploads/ is
never created — watched at 200ms through a whole run. So the chunk is
dropped inside HandleChunk before the write is posted.

Seven theories are recorded as REFUTED with how, including one I
briefly believed and wrote into a commit message: `!seq` does not
reject seq==0, since std::optional::operator bool IS has_value(), and
cb_input_dispatch_mouse.cc depends on that for coordinates that are
legitimately zero.

The finding names its own next step, per this directory's contract:
eee0fae logs every inbound frame type and both silent drop paths, so
one run should answer in a line what two rebuild cycles could not
answer from outside.

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

fix(upload): the file IO ran on the only task runner shape with no precedent ... 60e9c992ed

The chunk reached HandleChunk, passed every guard, was posted to the
receiver's own SequencedTaskRunner — and the file never appeared. Not
once, at 10 ms polling, across three image rolls.

That runner was built in the constructor with
base::ThreadPool::CreateSequencedTaskRunner, and it was the ONLY use
of that call anywhere in capture/. The download delegate — which
writes files in this same process and works — uses
base::ThreadPool::PostTask with explicit traits, and its own comment
says why: "no in-tree precedent is a cost paid hours later in the
build lane". It was, three times.

So all five file operations now use base::ThreadPool::PostTask /
PostTaskAndReplyWithResult with one shared constexpr TaskTraits,
matching cb_download_manager_delegate.cc:98 exactly. The traits form
is the one from base/task/task_traits.h's own doc comment.

Dropping the sequence is not free, and the comment now states what
actually makes it safe rather than hand-waving about O_APPEND (which
would not have been true for the first chunk — WriteFile truncates).
Two chunks cannot be in flight at once: writes are posted only from
HandleChunk on the UI sequence, and the seq check rejects any chunk
arriving before the previous one is counted. The client is serial too.
If either changes, this needs a sequence again, and the comment says
so.

Still UNVERIFIED: the lane has not run on this, and whether it fixes
the vanishing chunk is exactly what the next roll will say. The
diagnostics from eee0fae stay in — they are what turned "the upload is
broken" into "the chunk arrives, passes every guard, and the write
does not happen", which is what made this the obvious suspect.

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

docs(findings): the diagnostics answered it — the chunk arrives, the write does not ... 546f14bfb4

Updates the open finding with what the eee0fae logs showed on the live
guest: file_upload_chunk arrives (139 bytes), no drop warning fires, so
HandleChunk reached its post — and the file still never exists at 10ms
polling, in a directory that mkdir+touch prove writable.

That narrows it to the task runner, and names the one thing about this
code that had no precedent anywhere in capture/. 60e9c99 changes it.

Deliberately NOT marked fixed. The next roll says whether it is.

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

fix(upload): file_upload_end hashed the file before the chunk had been written ... 24ebd98010

THE cause. The task-runner theory was wrong — swapping the sequenced
runner for base::ThreadPool changed nothing, same failure, and that is
recorded rather than quietly dropped.

The client sends the last chunk and `end` back to back. The guest
handles both on the UI sequence within microseconds, while the chunk's
WRITE is a pool task that has not run yet. So HandleEnd hashed a file
that did not exist and replied "could not read the file back" — a
truthful description of a consequence that names no cause.

It was invisible from both ends, which is why it took three rolls:
OnChunkWritten's reply landed AFTER OnFinalised had erased the map
entry, so its `find` failed and it returned silently. The only
evidence anywhere was end's own error message.

Uploads now carry writes_in_flight. HandleEnd defers when it is
non-zero (setting end_pending and saying so in the log), and the last
write's reply finalises instead. FinaliseUpload is split out so both
paths share it.

OnChunkWritten's "entry already gone" branch now LOGS rather than
returning silently — that silence is precisely what hid this, and it
is the same rule as the chunk-drop paths in eee0fae: a guard whose
skipped path is silent is not a guard.

Also in this commit: the gateway's /api/current-url now returns the
page TITLE alongside the url. cdpTarget has parsed Title since it was
written and currentURL threw it away, so a streamed browser could show
an address and nothing else. Additive, so an older client is
unaffected; mutation-checked by returning an empty title.

UNVERIFIED: the lane has not run on this. What it fixes is measurable
though — tests/interactive --only uploads should reach 9/9.

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

fix(upload): restore two function bodies a scripted edit ate, and lint for it ... c94f10e416

The lane caught this in ~13 minutes:

  ld.lld: error: undefined symbol:
    cloud_browser::CbFileUploadReceiver::OnFinalised(...)

My HandleEnd rewrite replaced a range that ran from one function to
the next and took two OTHER bodies with it. OnFinalised and
ResolveChooserWith were still declared, still called, defined nowhere.
Both restored verbatim from 546f14b.

`make lint-cxx` was clean — it checks includes. `make verify` was
clean — it cannot compile capture/. So a defect a script can find in
milliseconds cost a full lane cycle, on the one component where a
cycle is 20 minutes plus an image roll.

So: lint-cxx-orphan. It is strictly stronger than the linker, which
only notices when a CALL exists — this also catches a method declared,
defined nowhere, and not yet called, which links today and breaks
whoever adds the first caller.

Checked against the whole tree before landing (CLAUDE.md's rule). It
found two false positives on the first run — RTC_GUARDED_BY and
ABSL_GUARDED_BY, thread-safety macros that trail a member declaration
and parse exactly like a call — which are now excluded by suffix.
Clean across all 53 header/impl pairs after that. Negative control run
both ways: deleting OnFinalised's body fails it at the right symbol,
restoring it passes.

It deliberately under-reports (skipping inline bodies, pure virtuals,
= default/delete, templates) because a false positive costs more trust
than a missed defect costs time.

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

feat(client): the browser tab says what page you are looking at ... cf9b09c565

The gateway parsed the page title off DevTools /json into
cdpTarget.Title and threw it away, so the client had an address and
nothing else to show — a URL bar with no page behind it, and a viewer
tab reading "chromeless — v0 client" for every session.

/api/current-url now returns it (additive, so an older client reading
only `url` is unaffected) and the client sets document.title from it.
currentPage() reads the field defensively and currentUrl() is kept as
a one-line wrapper, because this stack rolls in two pieces — the
bundle is baked into the gateway image, the worker is separate — so a
client newer than its gateway is the normal case, not an edge one.
That skew is a test, not an assumption.

A non-string title is dropped rather than rendered: the body comes
over the network and "[object Object]" in the tab is worse than no
title. Mutation-checked.

Four client tests, one Go test, 333 client tests passing.

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

docs(findings): root cause — end hashed the file before the write ran ... 0c85511235

Records the answer and, deliberately, the theory that was wrong: the
task-runner swap produced an identical failure and fixed nothing.
"The previous theory was refuted" is the useful half of a diagnosis
and it disappears if only the eventual fix gets written down.

The cause is a race. The client sends the last chunk and `end` back to
back; the guest handles both on the UI sequence in microseconds while
the write is a pool task, and HandleEnd never waited for it. Two
silent paths hid it from each other — OnChunkWritten's "entry already
gone" branch returned with no log, so the write's outcome was never
reported at all, and end's "could not read the file back" was the only
evidence anywhere.

Also records the cost honestly: five build-and-roll cycles, and all
three real defects in this feature were SILENT PATHS rather than wrong
computations. On a component whose feedback loop is 20 minutes plus an
image roll, a silent branch costs a full cycle every time it is hit.
The per-frame log should have been the first change, not the fourth.

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

docs: on a lane-bound component, log the wire before theorising about it ... fc84b0413f

The silent-guard rule already in this file cost five build-and-roll
cycles today, so it gets the concrete instance and the multiplier.

All three real defects in the file-upload path were silent paths, not
wrong computations, and two of them hid each other: the write's own
outcome was never logged, so every theory was about why the write
failed when it had simply not run yet.

One LOG(INFO) per inbound frame answered in a single run what four
cycles of static reasoning could not. It was the fourth change made;
it should have been the first, and that ordering is the point.

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

docs(interactive): "65 of 65" is stale, and a bare total is the wrong criterion ... 9f7e8e7719

The suite grew to 91 check() sites today — uploads is new (12) and stats
went 6 → 13 (audio reception, the unmute control, the HUD) — so the
README's headline number was about to send the next reader looking for
a regression that is just arithmetic.

Replaced with the criterion that actually holds: ZERO FAILED lines.
Suites skip whole blocks when a preflight says the guest binary or the
served gateway bundle predates a feature, and those skips are reported
rather than silently counted, so the printed total legitimately varies
between runs on different images.

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

diag(upload): the write SUCCEEDS and the read of the same path fails ... b1cf3697ae

The race fix worked and the upload still fails, which is progress: the
log now proves what the previous four cycles could not.

  <- file_upload_chunk
  <- file_upload_end
  end ... arrived with 1 write(s) still in flight; finalising when they land
  failed (write_failed): could not read the file back

The deferral branch was taken, so FinaliseUpload's only possible caller
was OnChunkWritten — which reaches it only on ok==true (ok==false goes
to AbandonUpload with a different message). Therefore AppendChunk
returned TRUE and HashFile's ReadFileToString then returned FALSE, on
the same path, about a millisecond apart.

That is a new and much narrower fact, and three explanations remain:
the file is written and removed in between, the write returns true
without creating a file, or the two paths are not the same string.

So this commit distinguishes all three in ONE run rather than three:

- AppendChunk logs which call it made, whether it returned true, and
  base::GetFileSize of the path immediately after — a write that
  returns true and leaves no file is exactly the shape being chased.
- HashFile logs, on failure, whether the file is ABSENT or present and
  unreadable. Those are different bugs (a delete racing the read
  versus permissions) and "could not read the file back" covers both.
- FinaliseUpload logs the path it is about to hash, so it can be
  compared byte-for-byte with the one AppendChunk wrote.

This is the lesson from the last four cycles applied deliberately:
instrument every branch that could explain the observation, rather
than pick the most likely one and spend a roll on it.

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

fix(upload): a use-after-move reported every successful upload as a failure ... f9deaaa7ef

THE cause, found by instrumenting rather than theorising:

  CV2-UPLOAD: WriteFile -> ok, 27 bytes; file is now 27 at .../Uploads/...
  CV2-UPLOAD: finalising ...; hashing .../Uploads/...
  CV2-UPLOAD: upload ... failed: could not read the file back

The write succeeded. The file was 27 bytes on disk. And the ERROR I had
added to HashFile's read-failure path never printed once — so the read
succeeded too. Both halves worked and the caller still saw failure.

    self->OnFinalised(std::move(id), std::move(p),
                      std::move(result.first), result.second,
                      !result.first.empty());        // argument 5

`result.first` is moved into argument 3 and read in argument 5. C++
leaves argument evaluation order UNSPECIFIED; on this toolchain
argument 3 won, so `ok` was computed from a moved-from empty string.
Every upload reported failure while the file sat correct on disk.

Compute the flag into a named local first. Never read a value in the
same call that moves it.

Five image rolls went into this, and the reason is that the error
message named a cause that had not happened: "could not read the file
back" sent me through the filesystem, the task runner, and a genuine
end/write race (that one was real, and its fix stands) before the line
itself. So the message is now `hash_failed` and says only what it
knows, with the read logging its own failure separately. The spec's
code table records the old lie.

Two things landed to make the class cheap next time:

- `lint-cxx-use-after-move`. Only the UNSEQUENCED case — a move and a
  read of one name inside a single argument list. A first version that
  ignored statement boundaries produced 14 findings, ALL false
  (`auto x = std::move(m_); m_.clear();` is legal and deliberate), and
  a second matched `chunk_bytes` as a read of `bytes`. Both fixed;
  clean across 76 files, negative control reintroduces the bug and it
  fails at the right line.
- The AppendChunk / HashFile / FinaliseUpload logging that produced
  the three facts above in ONE run, after four cycles of reasoning
  from the outside had produced none.

UNVERIFIED: the lane has not run on this. The uploads suite should
reach 9/9; that is the next roll.

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

fix(release): preserve build ancestry lost by batch B squash ... 8eb9599dd8

Merge the recorded f9deaaa7 guest source already integrated by PR109. Retain all current main files, including newer upload diagnostics, docs, and release pins. The resulting tree is identical to main; this restores auditable source ancestry without changing a deployed artifact.

triform-admin scheduled this pull request to auto merge when all checks succeed 2026-09-09 22:49:44 +00:00

triform-admin merged commit 8cf539ecb4 into main 2026-09-09 22:53:49 +00:00

triform-admin referenced this pull request from a commit 2026-09-09 22:53:49 +00:00

Merge pull request 'fix(release): retain source ancestry after the batch B squash' (#111) from agent/mapdb-provenance into main

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!111

No description provided.