Add fcvm balloon to read and set a running VM's balloon target

[ejc3]

fcvm balloon (--pid PID | --name NAME) [MIB] prints a running VM's balloon target and size as one JSON line, and sets the target first when MIB is given.

Follows: #1060

The Problem

--balloon attaches the device at boot, and since #1060 a snapshot cache hit sets its own target at restore. After that, the target of a running VM could be changed only by talking to Firecracker's API socket by hand, and nothing in fcvm said what the device was at.

The Solution

$ fcvm balloon --name web
{"target_mib":64,"actual_mib":64}
$ fcvm balloon --name web 96
{"target_mib":96,"actual_mib":64}

The target is what the device was asked to reach; the size is what the guest has given it so far. After a set the command prints right away, so the size is still on its way.

• Refused before any request, from the VM's state: a Cloud Hypervisor VM, by name (fcvm's client for it has no balloon call), and a target above the VM's memory, with both numbers.
• No device. Whether the VM has a balloon is read from GET /vm/config, which answers 200 either way, so a VM booted without --balloon is told it has no device and that one is attached only at boot. Nothing is inferred from a 400.
• A refused set. When Firecracker answers the PATCH with a 400 (the typed ApiRefusal from Share one snapshot between runs that differ only in the balloon target #1060), the error says what Firecracker refuses: a device the guest never activated, as with a kernel that has no virtio balloon driver. A timeout or any other failure says only what was being done.
• A set holds the VM's snapshot lock (the second commit, from the review of the first). Every snapshot of the VM holds the per-VM snapshot lock from its read of the balloon to its save: snapshot create, memory and disk-only, and podman run's and podman prepare's own snapshots. A pause does not keep a PATCH out, because Firecracker applies it to a paused VM. So a set that took no lock could land between the read and the save, and the snapshot recorded one target and saved a device at another; two sets could also interleave, and one reported the other's target. A set now takes that lock before its device check and holds it through the PATCH and the report. It waits up to 60 seconds, then fails and says the target was not set, because a snapshot of a large VM holds the lock for minutes. A report without MIB changes nothing, takes no lock and does not wait. A PATCH sent to the API socket by anything other than fcvm is outside the lock.
• A set while a workload initializes costs the run its startup snapshot (the third commit, from the review of the second). A startup snapshot is named for the target its workload initialized under (<key>-balloon<MIB>-startup, Share one snapshot between runs that differ only in the balloon target #1060), and a restore can set a target but cannot replay the initialization. A set that landed after a podman run --health-check had published its state and before the workload was healthy changed the memory the workload initialized with, and the run still saved that workload under the name of the target it started with, where later runs at that target restored it. The creator now reads the device before a startup snapshot and compares it with the target the run started with. When they differ, the run takes no startup snapshot, under either name, and logs why at info; later runs restore the shared pre-start snapshot and make their own. podman prepare fails instead and installs nothing, because the snapshot is what it was asked for. The comparison runs under the per-VM snapshot lock, which a set takes too, so a set cannot land between it and the save, and it runs before the pause, so a VM whose snapshot is declined is not paused. The pre-start snapshot needs no check: it is taken before the workload starts and is shared between targets. Sets are not refused and no lock is added. A target that was changed and changed back before the run turned healthy is not seen.

The command writes no state. What follows the new target and what does not:

Path	Follows the new target?	Why
The next snapshot of the VM: snapshot create, podman run's own pre-start snapshot, a disk-only capture	Yes	Since #1060 they read the device from the VMM (GET /vm/config), not from the state
podman run's own startup snapshot, and podman prepare's	Not taken	It is named for the target the run started with, so a run whose device is at another target when it turns healthy saves none, and podman prepare fails
A clone of that snapshot: its state, and its relaunch after a guest reboot	Yes	A restore records what the restored VMM reports, and a clone relaunches from its state
A disk-only clone of that snapshot	Yes	It cold-boots from the snapshot's record
The VM's own state (fcvm ls --json, config.balloon_mib)	No	It is written at boot and at restore and nowhere else, and this command does not write it
The VM's own relaunch after a guest reboot	No	A cold-booted VM relaunches from its launch config and a restored clone from its state; Firecracker has exited by then, so there is nothing to ask

The two "No" rows are the gap. Closing it needs a state write under the per-VM lock and a launch config that follows it, which is not in this PR.

Also in this PR: exec's lookup of a VM by --pid or --name moves to commands::common::load_vm_state and both commands use it; the text of what Firecracker refuses, and the test for a 400, are shared with the restore's PATCH; BalloonStats serializes, since it is the command's output. Docs: README CLI table, DESIGN.md command summary and a fcvm balloon section, and the --balloon help.

Test Results

Each test was watched failing without its subject. The reds are from this change over the first commit of #1060; every green step ran again after it moved onto the second. The second commit's reds and greens ran over #1060's third commit.

# the command's skeleton with none of its checks: make _test-unit FILTER="-E 'test(/.../)'"
  FAIL cli::args::tests::balloon_names_one_vm_by_pid_or_by_name
      --pid and --name were accepted together: BalloonArgs { pid: Some(4242), name: Some("web"), mib: None }
  FAIL commands::balloon::tests::a_cloud_hypervisor_vm_is_refused_by_name_before_any_request
      a Cloud Hypervisor VM was accepted
  FAIL commands::balloon::tests::a_target_above_the_vms_memory_is_refused_before_any_request
      a 1025 MiB target for a 1024 MiB VM was accepted
  FAIL commands::balloon::tests::a_vm_with_no_balloon_device_is_told_where_one_comes_from
      a VM with no balloon device was accepted
  FAIL commands::balloon::tests::a_failed_set_explains_a_refusal_and_nothing_else
  FAIL commands::balloon::tests::the_report_is_one_json_line
      left: "{\n  \"target_mib\": 96,\n  \"actual_mib\": 64\n}"   right: "{\"target_mib\":96,\"actual_mib\":64}"
     Summary 8 tests run: 2 passed, 6 failed

# MIB made a required argument
  FAIL cli::args::tests::balloon_mib_is_optional
      a report needs no MIB: ErrorInner { kind: MissingRequiredArgument, ... Strings(["<MIB>"]) ... }
     Summary 2 tests run: 0 passed, 2 failed   (balloon_names_one_vm_by_pid_or_by_name fails with it: its cases give no MIB)

The VM test, make _test-root FILTER="--retries 0 -E 'test(=test_balloon_command_sets_and_reports_the_target)'" STREAM=1, with the PATCH and the device check removed:

Error: `fcvm balloon --pid P 96` printed target 64 MiB
after `fcvm balloon --pid P 96` Firecracker reports the device at target 64 MiB
a snapshot taken after the target was set to 96 MiB records Some(64)
fcvm balloon ["--pid", "<pid>"] on a VM booted without --balloon: succeeded=false, stderr "... ERROR fcvm: Error: reading the balloon's target and size from VM 'balloon-cmd-clone-861942-0': Firecracker API error: 400 Bad Request - {"fault_message":"Internal VMM error: Failed perform action on device: Device not found"}"
fcvm balloon ["--pid", "<pid>", "96"] on a VM booted without --balloon: succeeded=false, stderr (the same error)
        FAIL [  24.838s] fcvm::test_balloon test_balloon_command_sets_and_reports_the_target

The second commit, make _test-root FILTER="--retries 0 -E 'test(=test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save) | test(=test_two_balloon_sets_each_report_their_own_target)'" STREAM=1, with the command setting without the lock:

Error: the snapshot records balloon Some(64) and the device it saved holds 96 MiB: a target was set between the snapshot's read and its save
        FAIL test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save
Error: the first set asked for 96 MiB and reported target 80 MiB
        FAIL test_two_balloon_sets_each_report_their_own_target

Its unit tests, each with one statement mutated:

# the bounded wait never gives up
  FAIL commands::balloon::tests::a_set_gives_up_while_the_snapshot_lock_stays_held_and_says_nothing_was_set
      a set with a 300 ms wait was still waiting for a held lock after 10s
# create_podman_snapshot gives the lock back as soon as it has it
  FAIL commands::podman::snapshot::tests::podmans_own_snapshots_hold_the_vm_snapshot_lock_through_the_save
      create_podman_snapshot does not keep the per-VM snapshot lock for the rest of the function

The third commit, make _test-root FILTER="--retries 0 -E 'test(=test_startup_snapshot_is_not_taken_after_the_balloon_target_was_changed)'" STREAM=1, first with the creator taking a startup snapshot without the comparison, then with only the restore loop asking for its startup snapshot at any target:

Error: run 1's balloon target was changed from 64 to 96 MiB before its workload initialized, and the run saved the startup snapshot 51642dabcc92-balloon64-startup, which later 64 MiB runs restore
        FAIL [  14.452s] fcvm::test_balloon test_startup_snapshot_is_not_taken_after_the_balloon_target_was_changed
Error: run 2 restored the pre-start snapshot, its balloon target was changed from 64 to 96 MiB before its workload initialized, and the run saved the startup snapshot 7a7501fc333d-balloon64-startup
        FAIL [  24.470s] fcvm::test_balloon test_startup_snapshot_is_not_taken_after_the_balloon_target_was_changed

Its source pins, each with one statement mutated:

# the comparison removed
  FAIL commands::podman::snapshot::tests::a_startup_snapshot_compares_the_balloon_target_under_the_vm_snapshot_lock
      create_podman_snapshot does not read the VM's balloon target before a startup snapshot
# the restore loop asks at any target
  FAIL fcvm::test_balloon_call_sites every_startup_snapshot_is_asked_for_at_the_target_its_run_started_with
      1 of 4 statements that keep a startup snapshot to the balloon target its run started with are gone.

Green, on the third commit (1-minute load on the host 34 to 68 from other work):

$ make _test-root FILTER="--retries 0 -E '<the new test, the command's three VM tests, and test_startup_snapshot_is_per_balloon_target>'" STREAM=1
        PASS [  22.923s] fcvm::test_balloon test_balloon_command_sets_and_reports_the_target
        PASS [  31.388s] fcvm::test_balloon test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save
        PASS [  24.483s] fcvm::test_balloon test_startup_snapshot_is_not_taken_after_the_balloon_target_was_changed
        PASS [  21.118s] fcvm::test_balloon test_two_balloon_sets_each_report_their_own_target
        PASS [  26.516s] fcvm::test_snapshot_clone test_startup_snapshot_is_per_balloon_target
     Summary [ 126.434s] 5 tests run: 5 passed, 1799 skipped

$ make _test-unit (the new pins with the snapshot-lock, snapshot-key and prepare unit tests)
     Summary [   0.322s] 26 tests run: 26 passed, 1420 skipped
$ make _test-unit (the module filter below)
     Summary [   8.263s] 567 tests run: 567 passed, 879 skipped

$ make fmt      # leaves the tree unchanged
$ make clippy   # clean

In that run the new test's first two runs logged Not taking the startup snapshot, and its third, whose target nobody changed, logged Startup snapshot created successfully.

Green, on the second commit:

$ make _test-root FILTER="--retries 0 -E 'test(=test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save) | test(=test_two_balloon_sets_each_report_their_own_target) | test(=test_balloon_command_sets_and_reports_the_target) | test(=test_snapshot_and_restored_clone_record_the_balloon_the_vmm_has)'" STREAM=1
        PASS [  23.106s] fcvm::test_balloon test_balloon_command_sets_and_reports_the_target
        PASS [  31.618s] fcvm::test_balloon test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save
        PASS [  19.992s] fcvm::test_balloon test_two_balloon_sets_each_report_their_own_target
        PASS [  21.974s] fcvm::test_snapshot_clone test_snapshot_and_restored_clone_record_the_balloon_the_vmm_has
        Summary [  96.697s] 4 tests run: 4 passed, 1797 skipped

$ make _test-unit (the module filter below)
     Summary [   8.342s] 565 tests run: 565 passed, 879 skipped

$ make fmt      # leaves the tree unchanged
$ make clippy   # clean

test_balloon_set_cannot_land_between_a_snapshots_read_and_its_save holds snapshot create with a failpoint between its read of the balloon (64 MiB) and its save, starts fcvm balloon --pid P 96 while it is held, and then restores a clone from the snapshot: the clone's device has to be at the 64 the snapshot records, and the set has to report 96 once the snapshot is done. test_two_balloon_sets_each_report_their_own_target holds one set between its PATCH and its report and starts a second with another target: each has to report its own.

Green, on the first commit, before it moved onto #1060's third commit:

$ make _test-root FILTER="--retries 0 -E 'test(=test_balloon_command_sets_and_reports_the_target) | test(=test_restored_clone_reboot_keeps_its_balloon) | test(=test_snapshot_and_restored_clone_record_the_balloon_the_vmm_has)'" STREAM=1
        PASS [  23.275s] fcvm::test_balloon test_balloon_command_sets_and_reports_the_target
        PASS [  32.589s] fcvm::test_reboot test_restored_clone_reboot_keeps_its_balloon
        PASS [  33.016s] fcvm::test_snapshot_clone test_snapshot_and_restored_clone_record_the_balloon_the_vmm_has
     Summary [  88.889s] 3 tests run: 3 passed, 1793 skipped

$ make _test-unit FILTER="-E 'binary(/^test_(hypervisor_api|balloon_call_sites|documented_make_targets|bench_fixtures|nested_harness_invariants|readme_examples|log_scan|ci_workflow_coverage|ami_hash_inputs|default_kernel_release|fuzz_chaos|state_manager)/) | test(/^firecracker::|^hypervisor::|^commands::|^cli::|^state::|^storage::/)'"
     Summary [   8.320s] 562 tests run: 562 passed, 879 skipped

$ make fmt      # leaves the tree unchanged
$ make clippy   # clean

test_balloon_command_sets_and_reports_the_target boots one VM with --balloon 64 and one without. It reads the report by PID and by name, sets 96 with the command, and reads 96 back from the command's own output, from Firecracker's GET /balloon/statistics and from a later report once the guest has brought the balloon to that size. It then checks that the VM's state still records 64, that a snapshot taken afterwards records 96, that a target above the VM's memory is refused, and that the VM without a balloon is told it has no balloon device (with and without MIB). The "no balloon device" answer is tested against the real VM: Firecracker itself answers a VM without the device 400 Bad Request ... Device not found (the last two lines of the red run above), and the command reads GET /vm/config first, so that VM is told it has no balloon device and where one comes from. The other two VM tests are in the run because exec's lookup moved (the reboot test drives the guest through fcvm exec --pid) and BalloonStats changed.

What is pinned and what is read: the "Yes" rows for snapshot create and for a clone's state are pinned by this test and by test_snapshot_and_restored_clone_record_the_balloon_the_vmm_has (#1060), and the state row by this test. The two reboot rows and the disk-only row are from reading the code. tests/test_balloon_call_sites.rs pins the statements the clone's reboot plan and the disk-only boot rest on; the cold-booted VM's relaunch from its launch config is not pinned.

Not run: a disk-only capture or one of podman run's own snapshots racing a set (they hold the same lock; the podman path is pinned by source and the disk-only path by #1060's pin), arm64, the nested profile's Firecracker, a VM whose guest never activated the device (the 400 explanation is covered by unit tests), Cloud Hypervisor (its refusal is a unit test on the VM's state), the full make test-root, and cargo audit and cargo deny (the Cargo files are unchanged).

Summary by CodeRabbit

• New Features
  • Added fcvm balloon to view a running VM’s balloon target and actual size, or set a new target.
• Bug Fixes
  • Balloon target changes are synchronized with snapshots, helping ensure snapshots reflect the VM’s current target.
  • If the target changes during workload startup, no startup snapshot is saved; podman prepare reports an error instead of installing an unusable snapshot.
  • Balloon targets above VM memory and attempts to set a target on VMs without a balloon device are rejected.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 093862a3-ee2f-4b87-8e8c-1427a172de0c

📥 Commits

Reviewing files that changed from the base of the PR and between 565611c and 9ac1318.

📒 Files selected for processing (15)

• DESIGN.md
• README.md
• src/cli/args.rs
• src/commands/balloon.rs
• src/commands/common.rs
• src/commands/exec.rs
• src/commands/mod.rs
• src/commands/podman/mod.rs
• src/commands/podman/snapshot.rs
• src/commands/podman/types.rs
• src/commands/snapshot.rs
• src/firecracker/api.rs
• src/main.rs
• tests/test_balloon.rs
• tests/test_balloon_call_sites.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Adds fcvm balloon to report or set a Firecracker VM’s balloon target. Balloon sets use the per-VM snapshot lock. Podman startup snapshot creation now checks that the balloon target matches the run’s starting target.

Changes

Balloon control

Layer / File(s)	Summary
CLI and command wiring src/cli/args.rs, src/commands/mod.rs, src/main.rs, README.md	Adds the fcvm balloon command with a PID or name selector and an optional MiB target. The command reports balloon target and actual size as JSON, or sets the target before reporting.
Balloon command and synchronization src/commands/balloon.rs, src/commands/common.rs, src/commands/exec.rs, src/firecracker/api.rs, tests/test_balloon.rs	The command rejects Cloud Hypervisor VMs, targets above configured memory, and VMs without a balloon device. Sets acquire the per-VM snapshot lock for up to 60 seconds. The shared lock API supports timed acquisition, and exec uses the shared VM-state loader. Tests cover command behavior, lock waiting, and overlapping sets.

Podman startup snapshots

Layer / File(s)	Summary
Balloon-aware snapshot creation src/commands/podman/snapshot.rs, src/commands/podman/types.rs	Adds snapshot requirements for any balloon target or the target recorded at run start. Target-specific snapshot creation checks the current target under the snapshot lock and returns a declined-snapshot outcome on mismatch.
Podman snapshot call sites and validation src/commands/podman/mod.rs, src/commands/snapshot.rs, tests/test_balloon.rs, tests/test_balloon_call_sites.rs, DESIGN.md, README.md	Pre-start snapshots accept any target; startup and prepare snapshots require the run’s starting target. A declined startup snapshot leaves the pre-start snapshot as parent. podman prepare reports a mismatch and states that nothing was installed. Integration tests cover target changes during workload initialization.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant CLI
  participant BalloonCommand as fcvm balloon
  participant VmState as VM state
  participant SnapshotLock as Snapshot lock
  participant Firecracker as Firecracker API
  User->>CLI: Select VM and optionally provide target
  CLI->>BalloonCommand: Dispatch command
  BalloonCommand->>VmState: Load VM state
  VmState-->>BalloonCommand: Return VM configuration
  alt Target provided
    BalloonCommand->>SnapshotLock: Acquire per-VM lock with timeout
    BalloonCommand->>Firecracker: Check device and set target
    Firecracker-->>BalloonCommand: Return balloon statistics
  else No target provided
    BalloonCommand->>Firecracker: Read balloon statistics
    Firecracker-->>BalloonCommand: Return balloon statistics
  end
  BalloonCommand-->>User: Print target and actual size as JSON

Loading

Merge Risk: ⚪ Minimal · up to 9ac13

The balloon command and startup-snapshot changes are ready to merge after normal checks; no actionable issue remains identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9ac13

The new command uses the selected VM’s existing control socket, validates requests and coordinates target changes with snapshots. No introduced security bypass was established. Risk remains bounded but not minimal because deployed access permissions and interrupted-request behavior are not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Each invocation targets one resolved VM within the configured data directory. Its direct effects are guest memory availability and that VM’s snapshot/cache eligibility, potentially disrupting workloads sharing the guest. The inspected command accepts neither an arbitrary socket endpoint nor a remote destination; cross-user exposure depends on existing filesystem and socket authority.

Trust Boundaries and Controls

• observed — Caller-controlled selectors resolve through shared state lookup, and the socket path derives from the selected vm_id. Backend and capacity checks precede client requests, while a successful configuration read confirms device presence before PATCH. Socket existence alone is not treated as authorization; connection success remains dependent on operating-system access controls.

Resilience and Maintainability Implications

• observed — Snapshot consistency coordination is cooperative, not an authorization boundary. The design explicitly excludes direct API-socket PATCH operations from the lock. Request deadlines and scoped lock ownership bound ordinary failures, but the inspected code does not establish whether an interrupted or timed-out PATCH can still complete inside the VMM.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 86.54% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 13 files. (2 skipped: 2…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding fcvm balloon to read and set a running VM’s balloon target.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-04T22:09:16.301476Z	9ac1318	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02adee92f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ejc3]

NOT-A-DEFECT: the review bodies on this pull request carry no finding of their own. Each finding they announce is an inline thread, and each thread is answered there with the test that was watched failing.

[ejc3]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 119fe747ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ejc3]

NOT-A-DEFECT: the review bot's summary comment was rewritten when this pull request's description changed, and it carries no finding. The findings on this pull request are inline threads, and each is answered there with the test that was watched failing.

[ejc3]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 9ac13184f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ejc3]

NOT-A-DEFECT: the review bot's summary comment was rewritten when this pull request's description changed, and it carries no finding. The findings on this pull request are inline threads, and each is answered there with the test that was watched failing.