refactor(files): make tunnel frame decoding host-testable (VAPP-83) #94

Merged

longjacksonle merged 1 commit from feat/VAPP-83-files-auto-refresh into main

2026-07-17 04:28:27 +02:00

longjacksonle commented

2026-07-17 04:24:18 +02:00

Owner

This does not fix VAPP-83. It is the instrumentation step, because the ticket's premise turned out to be wrong and the fix as specified would have been a guess. VAPP-83 stays open.

What the investigation found

Auto-refresh after a mutating op is already implemented on both sides, and has been since the file browser landed in cfc1942 (2026-06-12), a month before the ticket was filed:

The agent reports mkdir/mkfile/rm/rename/copy/move success as {action:"refresh"} rather than echoing the op (build_mkdir_reply and friends, vervain-agent::host::tunnel:1508,1537,1576,1600,1648,1683), and upload completion as uploaddone.
The pane already turned both into a re-ls of the current directory (files.rs:734, files.rs:758 before this change).
Those agent replies are present in every released tag (v0.3.0, v0.4.0, v0.4.1), so the macOS agent tested in VA-129 did send them.

So the ACs describe behavior the code already intends. Bolting a second refresh mechanism on top of a correct one would mask the real cause rather than find it, so this change makes the existing one observable instead.

Why the bug could hide here

The entire session driver is #[cfg(target_arch = "wasm32")]. cargo test and the host clippy pass never compile it, so the protocol had zero tests and was only ever type-checked by cargo check --target wasm32-unknown-unknown. live.rs deliberately keeps its control-channel decoding pure and wasm-free so it is testable; the file browser skipped that.

What this does

Splits the frame-to-decision mapping into a pure decode_files_frame -> FilesFrame (no wasm types, no signals), unit-testable on the server build, gated any(target_arch = "wasm32", test) so the server binary carries no dead code. Same split as live.rs. handle_text becomes a thin applier; behavior is unchanged.
Adds 10 tests pinning the contract, including the two that matter here: refresh asks for a re-listing, and uploaddone does too.
Mirrors frames to the browser console behind a runtime switch: localStorage.setItem("vc-files-debug", "1"), then reopen the tab. A runtime switch, not a build flag, so a reporter can capture frames on a normal deployment. Silent by default.

The useful result

The tests pass. That is the finding: the decision mapping is correct, so the stale listing is not a decode bug. The fault is either upstream (the refresh frame never arrives) or downstream (the re-ls reply never reaches the view). The console log now tells us which, in one repro.

The refactor also surfaced a latent gap: the exhaustiveness check caught a missing arm that the old catch-all had silently absorbed, and it only fired on the wasm target, which is exactly the blind spot described above.

To finish VAPP-83

Reproduce once with vc-files-debug on and grab the console:

-> {"action":"mkdir",...} then no <- {"action":"refresh"} means the agent side is at fault, and the ticket's "not agent-side" scoping is wrong.
<- {"action":"refresh"} then -> {"action":"ls",...} then <- {"action":"ls",...} with the new entry present means the frames are fine and the fault is in the view (a signal write from a JS callback not re-rendering).

Unrelated bug found

On the desktop build (dioxus-desktop, not wasm), attach returns a no-op FilesHandle stub (files.rs:452): mkdir_prompt, upload_from_input and navigate all do nothing and no WebSocket is ever opened, so the file browser is inert. terminal.rs and desktop.rs have the same shape, so it looks deliberate for the SSR build and merely inherited by desktop. Not touched here; happy to file it if it is not already known.

**This does not fix VAPP-83.** It is the instrumentation step, because the ticket's premise turned out to be wrong and the fix as specified would have been a guess. VAPP-83 stays open. ## What the investigation found Auto-refresh after a mutating op is **already implemented on both sides**, and has been since the file browser landed in `cfc1942` (2026-06-12), a month before the ticket was filed: - The agent reports mkdir/mkfile/rm/rename/copy/move success as `{action:"refresh"}` rather than echoing the op (`build_mkdir_reply` and friends, `vervain-agent::host::tunnel:1508,1537,1576,1600,1648,1683`), and upload completion as `uploaddone`. - The pane already turned both into a re-`ls` of the current directory (`files.rs:734`, `files.rs:758` before this change). - Those agent replies are present in **every** released tag (v0.3.0, v0.4.0, v0.4.1), so the macOS agent tested in VA-129 did send them. So the ACs describe behavior the code already intends. Bolting a second refresh mechanism on top of a correct one would mask the real cause rather than find it, so this change makes the existing one observable instead. ## Why the bug could hide here The entire session driver is `#[cfg(target_arch = "wasm32")]`. `cargo test` and the host clippy pass never compile it, so the protocol had **zero tests** and was only ever type-checked by `cargo check --target wasm32-unknown-unknown`. `live.rs` deliberately keeps its control-channel decoding pure and wasm-free so it *is* testable; the file browser skipped that. ## What this does - Splits the frame-to-decision mapping into a pure `decode_files_frame -> FilesFrame` (no wasm types, no signals), unit-testable on the server build, gated `any(target_arch = "wasm32", test)` so the server binary carries no dead code. Same split as `live.rs`. `handle_text` becomes a thin applier; behavior is unchanged. - Adds 10 tests pinning the contract, including the two that matter here: `refresh` asks for a re-listing, and `uploaddone` does too. - Mirrors frames to the browser console behind a runtime switch: `localStorage.setItem("vc-files-debug", "1")`, then reopen the tab. A runtime switch, not a build flag, so a reporter can capture frames on a normal deployment. Silent by default. ## The useful result **The tests pass.** That is the finding: the decision mapping is correct, so the stale listing is not a decode bug. The fault is either upstream (the `refresh` frame never arrives) or downstream (the re-`ls` reply never reaches the view). The console log now tells us which, in one repro. The refactor also surfaced a latent gap: the exhaustiveness check caught a missing arm that the old catch-all had silently absorbed, and it only fired on the wasm target, which is exactly the blind spot described above. ## To finish VAPP-83 Reproduce once with `vc-files-debug` on and grab the console: - `-> {"action":"mkdir",...}` then no `<- {"action":"refresh"}` means the agent side is at fault, and the ticket's "not agent-side" scoping is wrong. - `<- {"action":"refresh"}` then `-> {"action":"ls",...}` then `<- {"action":"ls",...}` with the new entry present means the frames are fine and the fault is in the view (a signal write from a JS callback not re-rendering). ## Unrelated bug found On the **desktop** build (`dioxus-desktop`, not wasm), `attach` returns a no-op `FilesHandle` stub (`files.rs:452`): `mkdir_prompt`, `upload_from_input` and `navigate` all do nothing and no WebSocket is ever opened, so the file browser is inert. `terminal.rs` and `desktop.rs` have the same shape, so it looks deliberate for the SSR build and merely inherited by desktop. Not touched here; happy to file it if it is not already known.

longjacksonle added 1 commit

2026-07-17 04:24:18 +02:00

refactor(files): make tunnel frame decoding host-testable (VAPP-83)

Check / fmt + clippy + build + tests (pull_request) Successful in 2m34s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

34e84a7632

VAPP-83 asks for auto-refresh after a mutating file op, but that is already implemented on both sides and has been since the file browser landed (cfc1942, a month before the ticket was filed). The agent reports mkdir/mkfile/rm/rename/copy/move success as `{action:"refresh"}` rather than echoing the op (`build_mkdir_reply` and friends in `vervain-agent::host::tunnel`), and reports upload completion as `uploaddone`; the pane already turned both into a re-`ls` of the current directory. Those agent replies are present in every released tag (v0.3.0, v0.4.0, v0.4.1), so the macOS build tested in VA-129 did send them. Adding a second refresh mechanism on top of a correct one would be guessing, so this change makes the existing one observable instead.

The whole session driver is `#[cfg(target_arch = "wasm32")]`, so `cargo test` and the host clippy pass never compile it: the protocol had zero tests and was only ever type-checked. Split the frame-to-decision mapping out as a pure `decode_files_frame` returning a `FilesFrame`, with no wasm or signal types, so it is unit-testable on the server build. This is the same split `live.rs` already uses for the control channel, gated `any(target_arch = "wasm32", test)` so the server binary does not carry it as dead code. `handle_text` becomes a thin applier of the decoded frame; behavior is unchanged.

Ten tests now pin the contract, including the two that matter for this ticket: a `refresh` frame asks for a re-listing, and `uploaddone` does too. They pass, which is itself the useful result: the decision mapping is correct, so the stale listing is not a decode bug and the fault is upstream (the frame never arriving) or downstream (the re-`ls` reply not reaching the view). The refactor also surfaced a latent gap the exhaustiveness check caught only on the wasm target, where the old catch-all arm silently absorbed anything unmatched.

To close the remaining gap without guessing, frames are now mirrored to the browser console behind a runtime switch: `localStorage.setItem("vc-files-debug", "1")` and reopen the tab. A stale-listing repro then shows exactly which frame is missing, which is what distinguishes "the agent never sent refresh" from "we sent the re-ls and dropped the reply". It is a runtime switch rather than a build flag so a reporter can capture it on a normal deployment; the default path stays silent.

This does not fix the reported symptom, and VAPP-83 stays open. Verified: fmt, wasm and server checks, and 100 unit tests pass. The six `clippy::unnecessary_sort_by` errors in `server_fns/mod.rs` predate this branch and reproduce on a clean `main` with the newer local toolchain.

#VAPP-83

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015KYXyUTntEcVJaTwn5pody
Signed-off-by: longjacksonle <longjacksonle@gmail.com>