Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deps/lz4/lz4frame.c`:
- Around line 1619-1620: The code computes srcStart = (const BYTE*)srcBuffer and
srcEnd = srcStart + *srcSizePtr even when srcBuffer can be NULL (seen when
calling LZ4F_decompress(dctx, NULL, &o, NULL, &i, NULL)), which causes undefined
behavior; instead of patching the vendored file inline, update/re-vendor to an
upstream LZ4 release that contains the fix, and while doing so verify that the
logic around LZ4F_decompress, srcBuffer, srcSizePtr, srcStart and srcEnd
guards/skips the pointer arithmetic when srcBuffer == NULL or *srcSizePtr == 0
(i.e., ensure the code checks for NULL/zero before computing srcEnd or uses a
safe branch), then run the decompression unit tests that exercise the NULL input
path to confirm the upstream fix resolves the UB.

In `@src/aof.c`:
- Around line 1014-1022: The code currently treats a short read (0 < nread <
sizeof(header)) as success by returning 0, which lets a truncated RDB header be
considered uncompressed; change this so any partial read is treated as an error:
after reading into header from fd (variables nread, header, sizeof(header), fd),
if nread == -1 restore read_errno and return -1 (as already done), and if nread
< (ssize_t)sizeof(header) also set errno to EIO (or restore read_errno if
appropriate) and return -1 so the caller will treat the probe as a failure and
trigger the fallback path instead of accepting a truncated header.

In `@src/compression_stream.c`:
- Around line 818-820: The cap check can overflow because effective_len + len
may wrap; update the check in the push/feed logic (where effective_len is
computed from sdslen(t->feed_queue) - t->feed_head) to first guard against
addition overflow and oversized single inputs by verifying either len >
t->feed_cap or that effective_len is greater than t->feed_cap - len (or
equivalently SIZE_MAX - effective_len < len) before calling
streamReaderSetError(t, STREAM_READER_ERROR_IO); this ensures you never perform
wrapping addition and correctly detect when the new data would exceed
t->feed_cap.

In `@src/config.c`:
- Around line 2590-2596: The updateReplCompression apply-hook is performing an
irreversible action (markCompressedReplicasForDisconnect) during CONFIG SET,
which can be rolled back by configSetCommand; change updateReplCompression to
avoid side effects: instead of calling markCompressedReplicasForDisconnect
directly, record the intent (e.g., set a new flag or push affected replica IDs
into a pending list) and return success; then invoke
markCompressedReplicasForDisconnect only from the post-commit path in
configSetCommand (or a dedicated commit callback) after all option apply-hooks
succeed, using the new flag/pending list; keep the function signature of
updateReplCompression and use symbols updateReplCompression,
markCompressedReplicasForDisconnect, and configSetCommand to locate and wire the
deferred action.

In `@src/networking.c`:
- Around line 4598-4606: c->querybuf is being mutated with sdscatlen() which may
realloc the SDS and corrupt the thread-local shared pointer (thread_shared_qb)
if the primary is still using it; before appending
server.repl_stream_decode_buf, check if c->querybuf == thread_shared_qb and if
so call sdsnewlen()/sdsdup to make a private copy (or otherwise allocate a fresh
SDS) and assign it to c->querybuf, then perform the sdscatlen() append and
update c->querybuf_peak; refer to c->querybuf, server.repl_stream_decode_buf,
thread_shared_qb and the decompression path to implement this guard.

In `@src/rdb.c`:
- Around line 3695-3698: The conditional is checking the wrong streaming flag:
replace the RIO_FLAG_STREAMING_COMPRESSION test with
RIO_FLAG_STREAMING_DECOMPRESSION so load-side streaming-decompression hits the
special notice; update the branch that currently reads "(rdb->flags &
RIO_FLAG_STREAMING_COMPRESSION) && (rdb->flags & RIO_FLAG_SKIP_RDB_CHECKSUM)" to
use RIO_FLAG_STREAMING_DECOMPRESSION, keeping the RIO_FLAG_SKIP_RDB_CHECKSUM
check and the serverLog(LL_NOTICE, ...) calls unchanged (refer to rdb->flags,
RIO_FLAG_STREAMING_DECOMPRESSION, RIO_FLAG_SKIP_RDB_CHECKSUM, and serverLog).

In `@src/replication.c`:
- Around line 4042-4049: The code only advertises compression when
use_diskless_load is true, making server.repl_compression a no-op for disk-based
syncs; change the branch to advertise REPLICA_CAPA_COMPRESSION_STR whenever
server.repl_compression is enabled (i.e., check server.repl_compression instead
of use_diskless_load) so the capability is sent for all full-sync/PSYNC flows;
update the block that sets argv/ lens/ argc (the one referencing
use_diskless_load, REPLICA_CAPA_COMPRESSION_STR and server.repl_compression) to
add the capability whenever server.repl_compression is true and leave any
diskless-specific logic unchanged.
- Around line 216-219: replRefreshDecompression currently drops failures from
replInitDecompression; change replRefreshDecompression to propagate errors by
returning an int (return C_ERR on failure, C_OK on success), call
replDestroyDecompression() then check the return value of
replInitDecompression() and return C_ERR if it fails, and update all new call
sites (those that run handshake/full-sync immediately after calling
replRefreshDecompression) to check its return value and abort the sync session
on C_ERR; keep references to replInitDecompression, replDestroyDecompression,
replRefreshDecompression and use the C_ERR/C_OK constants so callers can act
accordingly.

In `@src/rio.c`:
- Around line 506-509: In rioReadPartial, before calling r->read_some, ensure
the computed bytes_to_read (the min of len and r->max_processing_chunk) is not
greater than SSIZE_MAX; if it is, reject the request and return an appropriate
error instead of invoking r->read_some to avoid backend-dependent truncation/
sign issues. Update the logic around bytes_to_read (used in the existing
computation) to perform this SSIZE_MAX bound check and return an error (and set
errno consistently, e.g. EINVAL/ERANGE) when exceeded; also include the required
header for SSIZE_MAX if not present.

In `@src/server.c`:
- Around line 6654-6665: The compression_ratio calculation is inverted: change
the expression that now computes (uncompressed / compressed) to compute
(compressed / uncompressed) instead, guarding against division by zero. In the
printf-style block that builds the stats string (reference symbols:
"compression_ratio=%.2f", compressionAlgoName(REPL_COMPRESSION_ALGO),
replica->repl_data->repl_compressed_bytes_total,
replica->repl_data->repl_uncompressed_bytes_total), replace the ternary that
returns (double)repl_uncompressed_bytes_total /
(double)repl_compressed_bytes_total with one that returns
repl_uncompressed_bytes_total == 0 ? 0.0 :
(double)replica->repl_data->repl_compressed_bytes_total /
(double)replica->repl_data->repl_uncompressed_bytes_total so compression_ratio
reflects compressed/uncompressed safely.
- Around line 6650-6669: The INFO path reads replication compression counters
(repl_compressed_bytes_total, repl_uncompressed_bytes_total,
repl_compression_errors, repl_compression_cpu_usec,
repl_compression_pending_drains, repl_compression_thread_switches) while IO
threads update them in writeToReplicaCompressed/postWriteToReplica, creating a
data race; fix by making these counters atomic (e.g., _Atomic size_t or
atomic_ullong) or by protecting updates/reads with the replica lock and then use
atomic_load (or the lock) when building the INFO string in src/server.c; update
all increment/+= sites in writeToReplicaCompressed/postWriteToReplica to use
atomic_fetch_add (or acquire the same lock) and replace direct reads in the INFO
formatting block with atomic loads (or locked reads) to ensure race-free access.

In `@src/server.h`:
- Around line 1281-1288: The lifetime replication counters use size_t/long long
and will wrap on 32-bit builds; change repl_compressed_bytes_total,
repl_uncompressed_bytes_total, repl_compression_errors,
repl_compression_pending_drains and repl_compression_thread_switches from size_t
to uint64_t and change repl_compression_cpu_usec from long long to uint64_t
(leave last_processed_tid and affinity_tid as int), ensure the header includes
<stdint.h> and update any INFO/printf format specifiers or casts to use
PRIu64/uint64_t where these fields are printed; apply the same replacements to
the other identical field group mentioned in the comment.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 212-220: Update the two actions/checkout steps (the "Install
libbacktrace" checkout using uses: actions/checkout@de0fac2e4500d... and the
"Checkout Valkey" checkout with the same uses) to include persist-credentials:
false under their with: blocks to prevent Git credentials from being persisted;
ensure you add the persist-credentials: false key alongside the existing
repository/ref/path keys and keep YAML indentation consistent.

In `@src/compression.h`:
- Around line 47-55: Add brief function-level comments for
compressionAlgoSupportsStreaming, compressionAlgoName, streamCompressorInit,
streamCompressorDestroy, streamDecompressorInit, and streamDecompressorDestroy
that describe each function's contract: the meaning of return values (e.g.,
true/false or 0/error codes), who owns any returned pointers or resources,
lifecycle expectations (caller allocates/initializes struct before/after call,
who must call Destroy), and any error conditions; place these comments
immediately above each declaration in the header so the public API is
self-describing and documents why the functions exist as well as how callers
must use them.

In `@src/rdb.c`:
- Around line 3151-3192: Add short contract comments for rdbInputStreamInit,
rdbInputStreamPrepare, rdbInputStreamValidateEnd, and rdbInputStreamDestroy
describing the required call order (Init() to set up the struct, optional
Prepare() to wrap raw_rio into a decompressor which may replace rdb_rio and set
RIO_FLAG_SKIP_RDB_CHECKSUM, ValidateEnd() to be called after reads to verify the
decompressor stream, and Destroy() to free the decompressor), the ownership
rules (input does not take ownership of the provided raw_rio; Prepare()
initializes internal decompressor state that must be cleaned up by Destroy()),
and return/behavior expectations (Prepare() returns DECOMPRESS_RIO_INIT_* codes,
Destroy() is idempotent, ValidateEnd() returns C_OK/C_ERR only when
initialized). Include these comments immediately above the definitions of
rdbInputStreamInit, rdbInputStreamPrepare, rdbInputStreamValidateEnd, and
rdbInputStreamDestroy and reference the struct fields input->raw_rio,
input->rdb_rio, input->decompressor, and input->initialized.

In `@src/rdb.h`:
- Around line 190-197: Add a clear declaration-level lifecycle contract comment
for the rdbInputStream type describing ownership and required call order: state
that callers own the struct, which fields are initialized by the creator vs by
Init/Prepare, and that the API must be used in order Init -> Prepare ->
ValidateEnd -> Destroy (with Destroy cleaning up raw_rio/rdb_rio/decompressor if
initialized). Reference the rdbInputStream type and the lifecycle functions
(Init, Prepare, ValidateEnd, Destroy) and document when initialized boolean is
set/cleared, which function takes/returns ownership of rio pointers, and that
calling functions out of order is undefined to prevent misuse.

In `@src/replication.c`:
- Around line 90-219: This change touches replication.c (functions like
markCompressedReplicasForDisconnect, replBalanceAffinity,
replicaInitCompressionOnPsync, replInitDecompression, replDestroyDecompression)
which per project policy requires an architecture-level review; please add
`@core-team` to the PR/reviewers and include a short justification referencing the
altered replication protocol negotiation, state transitions, and IO-thread
interaction so the core team can perform the requested architectural review
before merging.

In `@src/server.h`:
- Around line 3048-3055: Add brief header documentation above the new
replication-compression API declaring which functions operate on per-replica
state vs global/singleton state, the expected call order/lifecycle (e.g.
replInitCompression/replDestroyCompression per-client,
replInitDecompression/replDestroyDecompression once for the process, when to
call replRefreshDecompression and markCompressedReplicasForDisconnect), and the
return semantics (which functions return 0 on success / non-zero on error and
whether callers own/must free any returned resources). Reference the exact
symbols in the docs: markCompressedReplicasForDisconnect,
replInitCompression(client *c, compressionAlgo algo, int level),
replDestroyCompression(client *c), replBalanceAffinity,
replDecompressQueryBuf(client *c, size_t new_data_start), replInitDecompression,
replDestroyDecompression, and replRefreshDecompression so callers can quickly
see ownership and ordering.

In `@tests/integration/rdb-compression.tcl`:
- Around line 273-275: The test uses broad assertions like the catch/err +
assert_match "*Error*" pattern (e.g., the catch {r debug reload nosave} err
followed by assert_match "*Error*" $err) which is too generic; replace those
with precise checks that validate the expected failure reason—either call the
existing verify_log_message helper with the expected error/log substring or
change assert_match to match a specific error pattern (include the expected
text) for each scenario (also update the similar occurrences at the other
reported spots). Ensure you update the specific statements (the catch/err blocks
and their assert_match lines) so they assert the exact error or log message that
the test intends to trigger rather than the generic "*Error*".

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 221-229: Both `actions/checkout` steps in the workflow currently
persist the GitHub token in local git config, which is unnecessary for this job.
Update the `Install libbacktrace` checkout and the subsequent `Checkout Valkey`
step to set `persist-credentials: false` so the token is not left available to
the PR-controlled `make` and `./runtest` commands.

In `@src/compression_lz4.c`:
- Around line 49-52: The LZ4 init path in compressionLz4CompressorInit and
compressionLz4DecompressorInit should not assert on a NULL context and then
still report success; update these init functions to detect allocation failure
from the
LZ4F_createCompressionContext_advanced/LZ4F_createDecompressionContext_advanced
calls, return -1 on failure, and leave cleanup to
streamCompressorInit/streamDecompressorInit so the wrapper can handle the error
path correctly.

In `@src/compression_repl.c`:
- Around line 108-129: The no-progress path in replDecodeFeed() drops unconsumed
compressed input, so replDecompressQueryBuf() may discard a partial frame and
corrupt replication. Update replDecompressor/replDecodeFeed() to retain pending
bytes across calls (or otherwise report how many bytes were actually consumed)
when streamDecompressorFeed() returns consumed == 0 and produced == 0, and only
allow the caller to trim the query buffer by the true consumed amount. Keep the
behavior aligned with the stream reader contract used by compression_stream.c so
split headers/blocks are preserved until they can be decoded.

In `@src/compression_stream.c`:
- Around line 259-301: The stream writer does not stop accepting operations
after a failure, so later calls can proceed on partially advanced state. Update
streamWriterWrite, streamWriterFlush, and streamWriterFinish to check
writer->errored up front and return failure/no-op immediately, and ensure
streamWriterFinish only marks writer->finished after streamWriterFeedAndEmit
succeeds so a failed final emit stays sticky. Use the existing
streamWriterWrite, streamWriterFlush, streamWriterFinish, and
writer->errored/finished fields to keep all public writer operations rejected
once an error has occurred.

In `@src/unit/test_compression.cpp`:
- Around line 304-345: The current regression test in
streamCompressorFeedErrorRecovery only covers the fully pre-frame failure path,
so add a second subcase that forces streamCompressorFeed() to emit the LZ4 frame
header and then fail before completing the flush/write. Use the existing
streamCompressorInit, streamCompressorOutputBound, and streamCompressorFeed flow
to trigger the partial-write error, then retry on the same stream and verify the
output still round-trips correctly instead of continuing from a corrupted state.
Keep the test in the same GoogleTest fixture and extend the assertions around
stream_started and the retry behavior to cover this header-written recovery
path.

In `@src/valkey-check-rdb.c`:
- Around line 851-854: The checksum-logging branch in the RDB checker is using
the wrong transport flag and misclassifies decompressed inputs. Update the
condition in the RDB check logic around the RIO flag handling to key off the
decompression wrapper state instead of RIO_FLAG_STREAMING_COMPRESSION, so
compressed inputs are recognized correctly before falling through to the generic
skip message. Use the existing checksum decision block in the checker function
that logs via rdbCheckInfo as the place to make this change.

In `@tests/integration/repl-compression.tcl`:
- Line 1: The repl-compression integration suite is still registered only under
the generic repl/external:skip tags, so the new repl-compression tag in
server.tcl never controls these tests. Update the tag declaration in
repl-compression.tcl to use repl-compression as the top-level tag and remove the
custom repl-compression-suite deny-tag gating so --tags repl-compression and
--tags -repl-compression correctly include or exclude the suite.
- Around line 79-114: The test temporarily changes the primary’s
repl-compression setting in the “Replica with repl-compression lz4-stream and
disk-backed load also negotiates compression” case, but it only restores it on
the success path. Update the test around the $primary config set and $primary
config set repl-compression no calls to use a try/finally pattern so the
original setting is always restored even if any wait_for_condition,
assert_equal, or replicaof step fails.

In `@tests/integration/replication-aof-sync.tcl`:
- Around line 167-214: The new replication restart test is asserting RDB reuse
for a primary using lz4-stream compression, but the current AOF sync path still
falls back to BGREWRITEAOF for that case. Either update the AOF sync/reuse logic
in the runtime code path that handles `src/aof.c` so `wait_for_sync` can reuse
the synced RDB as the AOF base, or adjust this test in
`replication-aof-sync.tcl` to expect the fallback log and non-reuse behavior;
keep the checks around `wait_for_sync`, `log_file_matches`, and
`get_cur_base_aof_name` consistent with whichever behavior is intended.

---

Duplicate comments:
In `@src/compression_lz4.c`:
- Around line 91-129: The undersized-output handling in the LZ4 feed path is not
atomic because `LZ4F_compressBegin()` can partially emit the frame header before
later `offset >= output_capacity` checks return `-1`. Update
`compression_lz4.c`’s feed/compress routine so any output-capacity failure after
`LZ4F_compressBegin()` is treated as a committed partial write, and make retries
use a fresh compressor state instead of continuing from `cctx`/`stream_started`;
align the retry contract with the public feed behavior in `compression.h` and
the dispatch path in `compression.c`.