docs: reproducible verification for the OpenADS Plus PRs (#22/#23/#24) — automated-review findings are against stale revisions

[Admnwk]

What this is

A short verification report (docs/openads-plus/VERIFICATION.md) for the three OpenADS Plus backend PRs — #22 (PostgreSQL), #23 (MariaDB), #24 (ODBC) — so they can be reviewed against the code and the test suite rather than against the inline automated-review comments.

Why

The inline Gemini Code Assist comments on those PRs were generated against earlier revisions of each branch. Every "high-severity" item it raised — null-deref on pucField, SELECT * fragility, cross-schema mixing, missing locks, unescaped identifiers — is already handled at the current HEAD. Read literally, those comments describe a snapshot that predates the polish commits, not the code you would merge.

That puts a reviewer in an unfair spot ("the bot says there's a bug — do I merge or not?"). This report removes the guesswork.

The proof is reproducible in minutes

The clearest one (no database server needed) is ODBC, #24:

tools\scripts\build_msvc_x64_odbc.bat
build\odbc-msvc\tests\openads_unit_tests.exe      ->  528 / 528  (44,666 assertions)
pwsh tools\scripts\run_odbc_tests.ps1             ->    4 / 4    (throwaway Access .accdb, zero server)
pwsh tools\scripts\run_firebird_odbc_tests.ps1    ->    4 / 4    (real Firebird .fdb)

The report maps each flagged item to the exact guard at HEAD (file + the few lines that handle it), for all three PRs.

Note

Gemini Code Assist (consumer) is being retired — new installs blocked 2026-06-18, reviews ceasing 2026-07-17 — and its comments on these PRs predate the current HEAD. They should not block the merges; the code + suites are the ground truth.

Happy to add any fixture or test you'd like to see covered.

[gemini-code-assist]

Code Review

This pull request introduces a new Rust-based terminal and web monitor tool, openmonitor, for openads_serverd telemetry, alongside significant backend enhancements to transaction rollback handling, connection resolution, and CDX index page allocation. Feedback on these changes highlights a few critical issues: a bug in rollback_to_savepoint where touched tables are collected after truncating operations, a potential denial-of-service vulnerability in openmonitor due to uncontrolled memory allocation from network-provided frame lengths, blocking connection attempts in TcpStream::connect, and a potential out-of-bounds selection issue with selected_session upon refreshing the HTTP sessions list.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

In rollback_to_savepoint, touched is populated from tx_.ops() after calling tx_.truncate_ops_to(idx). This means touched will only contain tables that have remaining operations before the savepoint, and will completely miss any tables that were modified only after the savepoint (which are the exact tables whose changes were just rolled back and need their record buffers refreshed).\n\nTo fix this, collect the touched table handles from the operations being undone (from idx to the end of ops) before truncating the transaction operations.

    std::unordered_set<Handle> touched;\n    for (std::size_t i = idx; i < ops.size(); ++i) {\n        touched.insert(static_cast<Handle>(ops[i].table));\n    }\n    tx_.truncate_ops_to(idx);

[gemini-code-assist]

The frame length len is read directly from the network as a u32 and then used to allocate a vector of size len (vec![0u8; len]). If a corrupted or malicious packet specifies an extremely large length (e.g., u32::MAX), this will cause an out-of-memory (OOM) panic or exhaust system resources, leading to a Denial of Service (DoS).\n\nEnforce a reasonable maximum limit on the frame size (e.g., 16 MB) before allocating the payload vector.

    let len = u32::from_be_bytes([header[0], header[1], header[2], header[3]]) as usize;\n    if len > 16 * 1024 * 1024 {\n        return Err(anyhow!(\"frame length too large: {len}\"));\n    }\n    let opcode = header[4];\n    let mut payload = vec![0u8; len];

[gemini-code-assist]

TcpStream::connect uses the operating system's default connection timeout, which can be up to 2 minutes. If the target server is offline or dropping packets, this will block the calling thread (including the TUI event loop or the web server thread) for a very long time, leading to a poor user experience.\n\nUse TcpStream::connect_timeout with a reasonable timeout (e.g., 5 seconds) to fail fast if the server is unreachable.

    let addr = format!(\"{host}:{port}\");\n    let addrs = std::net::ToSocketAddrs::to_socket_addrs(&addr)\n        .with_context(|| format!(\"resolve {addr}\"))?;\n    let mut stream = None;\n    for socket_addr in addrs {\n        if let Ok(s) = TcpStream::connect_timeout(&socket_addr, Duration::from_secs(5)) {\n            stream = Some(s);\n            break;\n        }\n    }\n    let mut stream = stream.ok_or_else(|| anyhow!(\"failed to connect to any resolved address for {addr}\"))?;

[gemini-code-assist]

When the sessions list is refreshed, some sessions might have been closed, which can leave selected_session pointing to an out-of-bounds index. While the current rendering and action code bounds-checks safely, it can result in no session being highlighted or selected.\n\nClamp selected_session to the new sessions list size during refresh to keep the selection valid.

    fn refresh(&mut self, cfg: &MonitorConfig) {\n        self.inner.refresh(cfg);\n        self.last_refresh = Instant::now();\n        if !self.inner.http.sessions.is_empty() {\n            self.selected_session = self.selected_session.min(self.inner.http.sessions.len() - 1);\n        } else {\n            self.selected_session = 0;\n        }\n    }