docs(matrix): consolidate ten conflicting sdk-feature-matrix PRs into one code-verified refresh (LAB-1400) - #48
Merged
Merged
Conversation
…rified refresh (LAB-1400) Ten open PRs all edited sdk-feature-matrix.md; every merge invalidated the other nine. Sequencing them costs more than authoring the end-state once, so this regenerates the matrix from current SDK code rather than unioning the stale diffs — several PRs describe SDKs that have shipped further since July. The matrix is a trust surface (LAB-388, LAB-998), so the point of doing this from code is the falsehoods it surfaces. Three cells on main claimed capabilities no code supports: - Key rotation py/rs ✅ — rotate_key() returns NotImplemented (cachekit-core src/encryption/core.rs:492). This repo's own decisions/key-rotation.md already said ❌ fleet-wide and cited protocol#29 as the fix; #29 never merged, so a merged decision record pointed at an unmerged PR while the matrix kept lying. - cachekit-rs ::secure preset — does not exist; the preset is ::encrypted and secure() is a post-build accessor that errors without encryption. - cachekit-rs sync support ✅ — every op is async fn; the macro only expands on async functions. Also corrected against code and the registries: py no builder API, hardware acceleration surfaced by Python only, rs MSRV 1.85 (not 1.82), ts Node 22+ (not 20+), py 0.17.1, ts 0.1.5, rs feature-flag list. Recorded a new instance of the same class: py's max_retries config fields have zero consumers. New rows and sections carried over from the folded PRs: Retry, Graceful degradation, Cross-instance L1 invalidation (LAB-520), client-L1 SWR (LAB-728), Orjson serializer, Observability (LAB-275). Flipped on verified evidence: rs circuit breaker / retry / degradation / cold-miss single-flight (LAB-518), rs Workers locking + TTL (LAB-426), ts SWR on Workers via withExecutionContext -> ctx.waitUntil (LAB-751). spec/wire-format.md now names both CI enforcement points for the envelope vectors (LAB-423, protocol#25): this repo's verify.yml and cachekit-core's sha256-pinned tests/wire_format_vectors.rs. Supersedes protocol#25 #28 #29 #31 #32 #33 #35 #37 #40 #43.
… LAB-513 (LAB-1400)
Expert panel (bug-hunter / security / craftsman / pragmatism, high stakes)
contradicted four claims in the first commit. All four verified before fixing.
Release state vs branch state — the panel caught this commit committing the
very bug the ticket exists to fix. Tag cachekit-rs-v0.5.0 is 494d578
(2026-07-25) with default = [cachekitio, encryption, l1], no reliability
feature, and no reliability.rs or flight.rs in src/. LAB-518/728/729 all
landed after it, so six rs cells were ✅ for code no `cargo add` can reach.
They are now 🚧 with footnote 13 naming the tag and the missing files. This
also corrects main's pre-existing LAB-729 backpressure ✅.
Security corrections, all verified against code:
- cachekit-py DOES ship a live PyO3 KeyRotationState
(rust/src/python_bindings.rs:258, registered :394). start_rotation()
returns success and is_rotating() then reports True while nothing consults
it — an operator who reads that as a grace window and swaps the master key
loses every encrypted entry. The previous footnote said no binding exists.
- Python's encrypted read path is fail-OPEN by default
(EncryptionWrapper fail_closed=False, encryption_wrapper.py:113; settings.py
:225), not fail-closed as documented. On fingerprint mismatch or AES-GCM auth
failure the default is warn + recompute. Anyone using hard errors as their
wrong-key or tamper alarm needs CACHEKIT_ENCRYPTION_FAIL_CLOSED=true.
- LAB-513 is real and now asserted rather than declined: ts `secure` is an
unconditional alias, `secure = { wrap: (fn, o) => this.wrap(fn, o) }`
(cache-core.ts:832), every intent is typed SecureCache (cache.ts:87), and
encryption applies only `if (this.encryption)` (:486) — so secure.wrap on a
non-secure() cache stores plaintext silently. Declining to assert it was the
wrong call when two greps settle it.
Further accuracy fixes: three of four rs presets need the non-default `redis`
feature (only ::io builds on defaults); py SWR also requires an explicit ttl=
(wrapper.py:666) which presets never set; max_retries has no *operational*
consumer (one no-op validator branch at settings.py:252) rather than zero
reads; ts 0.1.5 shipped core 0.4.0 and LAB-751, so the rollout note no longer
contradicts the version table; footnote 3 was missing file-backend.json; ts
metrics are Node-only, CI-excluded from the Workers bundle.
Trimmed per the pragmatism filter: banner cut to a date plus a reversed-cells
list, footnote 8 and two audit-narrative paragraphs deleted, self-defense
clauses removed. Added the CHANGELOG entry the banner's pointer promised.
Rejected: relocating the pre-existing backpressure and Workers footnote essays
to decisions/, and deleting the pre-existing wasm bundle-size figures — all
pre-existing content outside this ticket's scope (matrix end-state + queue
cleanup); mixing a prose refactor into an accuracy fix would obscure both.
This comment has been minimized.
This comment has been minimized.
Contributor
WalkthroughThe PR updates the SDK feature matrix with August 2026 capability and release information. It adds published-artifact version-floor validation, records key-rotation and observability status, and documents wire-format vector verification and SDK rollout details. ChangesSDK matrix verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
This was referenced Aug 3, 2026
Closed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
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 `@sdk-feature-matrix.md`:
- Line 71: Remove the blank quoted lines at the affected blockquote locations in
sdk-feature-matrix.md, including lines 71 and 159, so each blockquote paragraph
remains contiguous and markdownlint MD028 is resolved.
- Around line 34-37: Update the version entries for cachekit-py, cachekit-rs,
cachekit-core, and cachekit-ts in the overview table to use the documented
minimum-version format with a trailing “+”, preserving the existing version
values and other columns.
- Line 9: Update the “Last updated” date in the document header to 2026-08-03 if
the evidence was collected on the review date; otherwise defer the merge until
the audit date and evidence match. Keep the existing LAB-1400 consolidation text
unchanged.
- Around line 300-301: Update the Rust L1 cache capability entry near the
existing LAB-728 reference to explicitly mark serve-stale and single-flight
background refresh as unreleased/main-only and absent from the published
cachekit-rs 0.5.0 crate, matching the qualification used elsewhere in the
document.
In `@spec/wire-format.md`:
- Around line 35-41: Update the CI enforcement statement in spec/wire-format.md
at lines 35-41 to state that cachekit-core decodes all vectors but performs
decode-and-re-encode byte-identity assertions only for canonical *_bin vectors;
update the corresponding wording in sdk-feature-matrix.md at line 274 to reflect
the same scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23a43ed3-922d-4583-b723-dd3ee67525f7
📒 Files selected for processing (4)
CHANGELOG.mddecisions/key-rotation.mdsdk-feature-matrix.mdspec/wire-format.md
…lifier, blockquote lint, re-encode scope CodeRabbit-Resolved: sdk-feature-matrix.md:9:Correct the audit date before CodeRabbit-Resolved: sdk-feature-matrix.md:71:Remove the blank quoted lines CodeRabbit-Resolved: sdk-feature-matrix.md:301:Mark the Rust SWR capability a CodeRabbit-Resolved: spec/wire-format.md:41:Correct the `cachekit-core` re
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
This comment has been minimized.
This comment has been minimized.
…s (LAB-1400)
The expert panel found the previous pass had introduced two new false cells of
the exact class it existed to remove. Both confirmed against published
artifacts, not branches, and both fixed.
B1 — TypeScript has NOT shipped the protocol-1.1 bin flip, on either path.
The previous revision claimed ts 0.1.5 ships it and that core-wasm@0.1.2 pins
core 0.4.0 so Workers carries it too. Verified from npm: @cachekit-io/cachekit
0.1.5 (published 11:15Z) has dependency pins byte-identical to 0.1.4's — exact,
caret-free pins on cachekit-core-ts@0.1.2 and cachekit-core-wasm@0.1.1. All
five platform .node binaries in core-ts@0.1.2 embed cachekit-core-0.2.0;
core-wasm@0.1.1 embeds 0.3.0. core-wasm@0.1.2 does embed 0.4.0 but published
at 14:28Z, 3h13m AFTER ts 0.1.5, so no published cachekit pins it. ts#91 bumped
the source pin without republishing the addon it consumes.
Scoped the consequence correctly: this is intra-SDK rollout skew, NOT a
cross-SDK interop break. protocol#11 makes auto-mode bytes SDK-internal, so py
writing bin cannot break a ts reader. The panel's framing overreached there.
B2 — the Rust reliability tier IS released and on by default. cachekit-rs
0.6.0 published 2026-08-03T14:58:16Z, 74 minutes before the previous pass's
final commit, whose message added an "unreleased" qualifier. Verified inside
the published .crate: src/reliability.rs, src/flight.rs,
tests/reliability_tests.rs, get_with_swr in src/l1/mod.rs, and
default = [cachekitio, encryption, l1, reliability]. Six cells flipped 🚧 → ✅;
footnote 13 now records the release and why the qualifier outlived its truth.
0.6.0 also resolves core 0.4, so rs now carries the bin flip.
B3 — SDK Overview versions are floors. CodeRabbit asked for this and I
rebutted it; the panel overturned the rebuttal and both were right. Footnote 4
said "version cells are floors" with no table qualifier, and rs sat at 0.5.0
against a published 0.6.0 on the day the doc stamped itself current.
Root cause was structural, not careless: the matrix was regenerated from main
against a registry snapshot that went stale before the final commit landed.
Recorded the method in decisions/matrix-version-verification.md — registry
metadata establishes which artifact is current, and where an embedded
dependency decides the claim, the .crate/.tgz gets opened. Added
tools/check-version-floors.py to verify.yml: fails on a bare X.Y.Z in the SDK
Overview table. Scoped to that table on purpose — versions elsewhere are exact
facts about specific artifacts and are correctly bare. Verified it passes clean
and fails on the reintroduced 0.5.0.
Also from the panel:
- Footnote namespace repaired. Markers 1-4 were each defined TWICE with
unrelated content, so half the evidence pointers resolved to the wrong note —
including the floors note B3 depends on. Protocol Compliance is now 14-17;
every marker defined exactly once (17 total).
- The two buried security facts are now Encryption table rows: the py
fail-OPEN-by-default tamper/wrong-key mode, and secure-API enforcement
carrying the LAB-513 cache.secure.wrap alias (CWE-311). While writing the
first row I nearly asserted rs and ts both "fail closed" unverified — checked
instead: rs propagates (client.rs:143), ts propagates from getEntry but its
wrap() degradation is a bare `catch {}` with no error-class check
(reliability/degradation.ts:13), so a decrypt failure degrades to an uncached
recompute wherever degradation is on.
- spec/wire-format.md's implementation-status note no longer contradicts the
matrix; it now names rs 0.6.0 and the ts embedded-core reality.
- The banner promised per-cell evidence "in the PR body" that wasn't there. The
per-PR fold ledger now lives in CHANGELOG.md, which the banner points at.
All seven local checks green: five vector suites, frame-crosscheck, and the new
floors guard. No test-vector or normative bytes changed.
Second panel round on the remediation. Two panelists independently landed the same CRIT: the TypeScript cell I added in 0aaf298 was the inverse of the code. Verified before changing anything. I wrote "propagates from getEntry, but wrap() degradation is a bare catch{} wherever degradation is on". Wrong in both halves: - getEntry does not propagate — it RETURNS this.run('get', ...) (cache-core.ts:405) with the decrypt inside the callback (:415), so an EncryptionError from a tampered payload or a wrong key travels into ReliabilityExecutor.execute -> withDegradation and is eaten by the bare catch (reliability/degradation.ts:13). get() returns null, a silent miss; through wrap() the function is re-executed and the result RE-STORED. Retry and the circuit breaker both rethrow, so degradation is the only swallower. - "wherever degradation is on" reads conditional; it is unconditional on every encrypted preset. degradationEnabled = config.degradation !== false (reliability/executor.ts:39), and secure()/production()/io() all pass degradation: true (intents-core.ts:186). Only minimal sets false, and minimal carries no encryption. - There is no failClosed option anywhere in cachekit-ts — zero hits under packages/cachekit/src. Python's CACHEKIT_ENCRYPTION_FAIL_CLOSED has no ts counterpart, so the only lever is reliability: { degradation: false }, which also gives up backend-outage degradation. Net effect: I had presented TypeScript as safer than Python on this row when it is strictly less remediable. Now stated as fail-OPEN by default and NOT switchable, with the mechanism in new footnote 8. That footnote also closes the numbering gap left by an earlier deletion, so markers are contiguous 0-17. Also from the panel, each verified: - client.rs:143 was not a decrypt site — it is `impl PartialEq for SwrToken`. I had read an awk-offset as a file line. Real propagation sites are client.rs:830 and :847. The claim was true; the pointer was falsifiable, which rule 5 of the new decision record exists to prevent. - The rs fail-closed mechanism was misattributed to SecureCache, which contains no degradation logic. The gate is the macro suppressing fail_open_arm under args.secure (cachekit-macros/src/lib.rs:439-451). - Footnote 13 overclaimed "no feature flags": macro-level graceful degradation and the automatic single-flight wiring come from the proc-macro, and macros = ["dep:cachekit-macros"] is not in default. Circuit breaker, retry, backpressure and L1 SWR are genuinely default-on; those two need --features macros. - My footnote renumber missed one reference — the pattern required "(" directly before "see", and one site reads "— see ... note 2)". It still pointed at the DynamoDB note, the exact defect the renumber existed to remove. Now 15. - Bare npm package names (cachekit-core-ts@0.1.2, cachekit-core-wasm@0.1.1) do not resolve on npm and contradicted the scoped form already used in note 1. Scoped to @cachekit-io/... in both the matrix and spec/wire-format.md. - The ts Graceful-degradation cell was still an unqualified ✅ while py and rs carried their caveats — the same burial this work is fixing. Now flags that it catches every error class and points at the new row. Panel confirmed clean: every published-artifact claim in the rollout table and footnote 13, the interop-mode scoping (interop values are plain MessagePack with no envelope, so the bin flip genuinely cannot cross SDKs — verified against wire-format.md and interop-mode.md, not asserted), CWE-311 as the right class for the secure.wrap alias, footnote 5's rotation warnings surviving the renumber, and check-version-floors.py both ways. Six vector/crosscheck suites plus the floors guard green. No test-vector or normative bytes changed.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
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 `@decisions/matrix-version-verification.md`:
- Line 8: Replace the future decision date in
decisions/matrix-version-verification.md:8 with the actual acceptance date, or
defer merging until after 2026-08-04. Also update sdk-feature-matrix.md:9 with
the actual audit date and sdk-feature-matrix.md:42 with the actual
registry-evidence collection date.
- Line 45: Restrict the floor-version wording to SDK Overview versions: update
“Every version in the matrix” in decisions/matrix-version-verification.md (line
45) and “Every version in this document” in sdk-feature-matrix.md (line 42) to
“Every SDK Overview version”; make no other changes, since exact embedded-core
versions remain artifact evidence.
In `@tools/check-version-floors.py`:
- Line 42: Rename the generator expression variable l in the start assignment to
a descriptive, unambiguous identifier while preserving the existing HEADING
matching behavior and Ruff compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 172c6464-6d2a-4d7a-b27c-219133d2fffe
📒 Files selected for processing (6)
.github/workflows/verify.ymlCHANGELOG.mddecisions/matrix-version-verification.mdsdk-feature-matrix.mdspec/wire-format.mdtools/check-version-floors.py
CodeRabbit was right that "every version in the matrix is a floor" is too broad. The document deliberately carries bare, exact versions elsewhere -- an embedded `cachekit-core-0.2.0` read out of a `.node` binary, a caret-free npm pin -- and those are artifact evidence under rule 2, not floors. The blanket wording reclassified them as floors, which would make them false, and invited a future reader to "fix" them by appending `+`. Both sites now say what the CI guard actually enforces, and say why the rest stay bare. Also renames the `l` loop variable flagged by ruff E741. Uses `raw` rather than the suggested `text_line` so the line stays under 88 chars without a wrap, and to avoid shadowing the enclosing `text` parameter.
This comment has been minimized.
This comment has been minimized.
cdbedba scoped floor semantics to the SDK Overview table, which CodeRabbit was right to ask for -- but the sentence I wrote to explain the rest of the document said bare versions elsewhere are "deliberate ... do not correct them by adding `+`". Read literally that is an instruction to STRIP the seven floors that legitimately live outside that table: the Compliance Status interop row (`PyPI 0.14.0+`, `crates.io 0.4.0+`, `npm 0.1.3+`, all governed by note 17), note 13's `cachekit-rs 0.6.0+`, and the Architecture Notes release bullets. Following it reopens the LAB-998 class -- a wrong interop ship-status version -- with no CI enforcement, since the guard polices only the Overview column. Two expert panelists found this independently. It is the same mechanical failure class the PR exists to close, committed by the commit meant to close it, which is now the fifth instance in this file. All three sites now state the real rule: a version answering "which release do I need" is a floor, a version that is evidence about one specific artifact stays bare, and neither is mechanically convertible into the other. The checker docstring no longer claims everything elsewhere is bare.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
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 `@decisions/matrix-version-verification.md`:
- Line 47: Update the statement on line 54 to say that
tools/check-version-floors.py rejects bare versions only within the SDK Overview
section of sdk-feature-matrix.md, rather than anywhere in the document. Keep the
surrounding distinction between CI enforcement and reviewer discipline
unchanged.
In `@sdk-feature-matrix.md`:
- Line 42: Update the registry-verification date in the document’s introductory
floor-version note to the actual evidence-collection date, 2026-08-03, and make
the corresponding date in the matrix-version verification record consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7707e4e0-d659-407d-ab10-4b0e4c62eccf
📒 Files selected for processing (3)
decisions/matrix-version-verification.mdsdk-feature-matrix.mdtools/check-version-floors.py
…est (LAB-1400)
The previous run escalated two questions to Ray and left four determinate
findings unfixed. Both questions turned out to be answerable here, and all four
are fixed. Withdrawing that escalation.
Q1 (dual-read) is settled by an executed test, not a judgement call. The claim
"a reader built against core <= 0.3.0 rejects bin" is FALSE.
cachekit-core/tests/dual_decode.rs is the canonical implementation's permanent
CI proof: assert_all_readers_decode (:69) deserializes wire into
StorageEnvelopeLegacy — a plain Vec<u8>, explicitly "the shape every pre-writer-
flip SDK shipped" — and dual_decode_matrix_against_bin_vectors (:161) runs it
over every *_bin twin, with bin8/bin16/bin32 width tiers. Legacy readers accept
bin. It also matches CHANGELOG.md ("Not a breaking change — dual-read is mutual")
and wire-format.md's own verified compatibility table, both of which the false
claim contradicted.
So the fleet-upgrade sequencing advice derived from it — "ts instances still on
0.1.2 will meet bin envelopes and reject them; sequence the republish before any
fleet upgrade" — was a manufactured migration risk. Removed. The ts core lag is
now stated as what it is: a forgone size saving on ts's own writes, with nothing
to sequence. spec/wire-format.md:104 corrected to match.
Q2 (the gate) needed a rewrite, not a decision. Reproduced two of the reported
defects: an empty version cell PASSED, and a valid backticked floor `0.6.0+` was
REJECTED. Rewrote the parser to key off the header row's Version column, treat a
GFM separator row as one whose every cell matches ^:?-{3,}:?$, strip markdown
emphasis before validating, and fail closed when the table cannot be located or
parsed — a guard that silently checks nothing is worse than none. Added
tools/test_check_version_floors.py, a 15-case mutation suite covering every leak
the panel reported plus the false positives; it runs before the guard in CI so
the checker cannot degrade to reporting OK. 15/15.
Also made the gate stop overclaiming. verify.yml and the decision record both
said it encoded "the one failure mode that recurred four times". It catches ONE
of the six incidents — the rs 0.5.0 snapshot in the Overview table. LAB-388 (a
tick on dead code), LAB-998 (a ship-status boolean) and the footnote regressions
are invisible to it. An overclaiming gate converts a known gap into an
assumed-safe one.
The four determinate findings, each verified first:
- ts key-rotation cell promised nonce-exhaustion detection the caller cannot
observe. NonceExhaustedError is raised inside encrypt(), which sits inside the
set callback, so withDegradation absorbs it. On the one condition where
continuing to encrypt risks GCM nonce reuse, nothing reaches the application.
- footnote 8 was scoped read-only while the identical mechanism swallows writes:
setEntry returns this.run('set', ...) with encrypt inside (cache-core.ts:478,
:487), so an encrypt/NAPI/key failure stores nothing while resolving as
success and every later wrap() re-executes origin forever.
Rather than patch a third path, footnote 8 now states the general mechanism
once — the executor wraps every operation, degradation catches every error
class — and derives read, write and nonce-exhaustion from it. Three rounds
each patched one path of one mechanism; this states the mechanism.
- rs `workers` was an unqualified tick that cannot compile as written. The
published 0.6.0 crate has default = [cachekitio, encryption, l1, reliability]
and src/lib.rs carries compile_error! for workers x l1 AND workers x
reliability, so `cargo add cachekit-rs --features workers` trips both; it needs
--no-default-features. New footnote 19, referenced from both Workers cells and
the two Backend Abstraction rows that credit rs Workers capabilities.
- footnote 5 misdirected on the file's most safety-critical reversal: three
cells cited 5 for Python's fail-open encrypted read path, but 5's numbered
sentence is "Key rotation ships in no SDK" and the fail-open evidence sat in an
unnumbered paragraph. Split out as footnote 18.
Added rule 6 to the decision record, which is the lesson three of the six
incidents share: for a behavioural claim, cite the executed test rather than a
mechanism you traced. Where none exists, trace the whole path including its
error handling and say which layer you checked.
Footnotes: 20 markers, each defined exactly once, all referenced. Seven
vector/crosscheck suites plus the floors guard and its mutation suite green. No
test-vector or normative bytes changed.
This comment has been minimized.
This comment has been minimized.
…B-1400) Fourth panel round. It CONVERGED on the headline items — two panelists independently confirmed the dual-read reversal is correct (core 0.3.0's compressed_data really is a bare Vec<u8>, byte-identical to StorageEnvelopeLegacy; both lagging ts artifacts lock rmp-serde 1.3.1; checksum never enters the flip; the size guards are length-identical) and confirmed footnote 19's compile_error! facts against the published crate. It then found two new CRITs, both mine, both security, both in the OVERSTATING direction this time. CRIT — I invented a GCM nonce-reuse risk that cannot happen. footnote 8 said nonce exhaustion is the condition "where continuing to encrypt risks GCM nonce reuse". The core fails closed: generate_nonce() returns NonceCounterExhausted once counter >= u32::MAX, and the counter is AtomicU64 *specifically* so it stays exhausted rather than wrapping — the doc comment at cachekit-core/src/encryption/core.rs:168-185 spells out that AtomicU32 would wrap and cause reuse, which is why it is u64. Reuse is unreachable. The real unobservable consequence is that every encrypted set() on that encryptor silently stores nothing for the rest of the process's life, and an operator seeing only a rising miss rate would reach for a master-key rotation that ships nowhere and destroys every entry. Corrected, and footnote 5's ts sentence corrected with it — it had said the missing signal is the one telling an operator "rotation is now mandatory", which is the wrong remedy. CRIT — footnote 19 recommended an invocation that silently disables encryption. It said the Workers build is `--no-default-features --features workers`, adding encryption "if wanted". With the feature off, CacheKitBuilder::encryption() and ::encryption_from_bytes() compile to stubs returning Ok(self) (client.rs:1019-1032), so the documented builder call succeeds, nothing errors, and the cache stores plaintext at rest. Only secure() is cfg-gated and fails loudly. The recommended invocation now includes encryption, the no-op stub is called out as CWE-311 (same shape as the LAB-513 row six lines above), and the Encryption table's Rust column cites 19 so its ticks carry the precondition. Also from the round: - The tamper row's ts cell said "fail-OPEN ... reads and writes", which reads as "stores plaintext on encrypt failure" — the throw precedes backend.set, so nothing is stored. Now "fail-OPEN on reads, silently drops writes". - The ts key-rotation cell had lost its 5 citation in the last commit, leaving the ts column's only rotation pointer aimed at a note with no rotation content. Cites 5 and 8. - footnote 18 attributed both fingerprint mismatch and AES-GCM auth failure to EncryptionWrapper(fail_closed=False); that flag gates the fingerprint pre-check, and the auth-failure policy is handle_decrypt_failure (cache_handler.py:1307/:1317, resolved :550-552). Conclusion was right, the citation was half-right. Both cited now. - footnote 8's opening claim "*every* cache operation runs inside ReliabilityExecutor.execute" was over-general — L1 hits short-circuit before it and acquireLock sits outside. Scoped to operations that reach the backend. Worth noting the direction: having failed twice by being too narrow about this mechanism, I overshot into too broad. - check-version-floors.py rejected a floor carrying this document's own footnote markers (0.6.0+¹⁷ reported as "a bare snapshot"), and silently skipped a second table under the same heading. Both fixed — superscripts are stripped with the other decoration, and a second table now fails closed rather than reporting OK on a section it cannot account for. Mutation suite 15 -> 17 cases, both gaps covered, 17/17. - The "catches one of six incidents" arithmetic did not add up in the decision record or verify.yml (1 + 4 invisible = 5). Now stated as one facet of one incident, with the other five itemised. Footnotes: 20 markers, each defined once, all referenced. Seven vector suites plus the guard and its 17-case suite green. No test-vector or normative bytes changed.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
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 `@tools/check-version-floors.py`:
- Line 46: Replace the literal Unicode dashes in PLACEHOLDER within
tools/check-version-floors.py:46-46 with the escaped sequences \u2014 and
\u2013, preserving existing placeholder matching. Update the test fixture in
tools/test_check_version_floors.py:57-57 to use \u2013 instead of the literal
dash; no other behavior changes are needed.
In `@tools/test_check_version_floors.py`:
- Around line 88-90: Update the subprocess.run call in the test helper to
explicitly pass check=False and retain a narrow # noqa: S603 on this fixed,
shell-free command invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f31000e-abd9-4615-be0b-380aac9ebda1
📒 Files selected for processing (6)
.github/workflows/verify.ymldecisions/matrix-version-verification.mdsdk-feature-matrix.mdspec/wire-format.mdtools/check-version-floors.pytools/test_check_version_floors.py
…AB-1400)
CodeRabbit's post-d488c35 round, both threads (ruff RUF001). Accepted — the
readability point is specific to this code, not a blanket rule:
PLACEHOLDER = re.compile(r"^[—–-]{1,3}$")
That character class holds U+2014 EM DASH, U+2013 EN DASH and U+002D
HYPHEN-MINUS, which are visually indistinguishable in a monospace diff, so a
maintainer cannot tell what the class actually matches. Now written with \u
escapes and a comment naming all three, consistent with the DECORATION regex
which already used escapes. Same for the en-dash test fixture and PHP_ROW's
em dashes.
Also reworded the two f-string error messages that carried a prose em dash, so
both files are RUF001-clean at source level rather than clean-except-for-two.
Verified by tokenising the raw source (RUF001 reads source text, not
AST-resolved values — an early check of mine conflated the two and reported
false hits): 0 ambiguous characters in code string literals across both files.
The two remaining live in a docstring and a comment, which are RUF002/RUF003
scope and were not selected.
Note ruff is not configured in this repo (no pyproject/setup.cfg, no ruff step
in verify.yml), so nothing enforced this; it is accepted on merit and to stop
the finding recurring.
Behaviour is unchanged and proven so: the 17-case mutation suite still passes
17/17, including the em-dash and en-dash placeholder cases that exercise exactly
the escaped class. Seven vector/crosscheck suites plus the guard also green. No
matrix, spec, test-vector or normative bytes touched.
While in the file, aligned the checker docstring's incident arithmetic with the
decision record (it still said the guard catches one of six with an enumeration
summing to five; it catches one facet of one).
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Contributor
Author
|
@27Bslash6 — ready for your signoff. Automated review is clean at head
Re-verified locally at I do not merge — that's yours. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR consolidates ten conflicting open SDK feature matrix PRs (protocol#25, #28, #29, #31, #32, #33, #35, #37, #40, #43) into a single, code-verified refresh of the SDK feature matrix. Rather than merging stale PR diffs, the matrix was regenerated directly from current SDK source code (LAB-1400).
Key Changes
Corrected Feature Cells (Reversals)
Several previously documented capabilities were found to be inaccurate when verified against actual code and have been corrected:
ZeroKnowledgeEncryptor::rotate_key()returnsNotImplemented, and cachekit-py's importableKeyRotationStatePyO3 binding "succeeds" while rotating nothing — flagged as a trust hazard with a strong operator warning.::securepreset and sync support (✅ → never existed): Rust is async-only; the encrypted preset is::encrypted, andsecure()is a post-build accessor.Unreleased Rust Reliability Tier
Six Rust reliability cells (circuit breaker, retry, graceful degradation, backpressure, stampede prevention, client-L1 SWR) are marked 🚧 unreleased: the code exists on
main(LAB-518/728/729) but is absent from the published 0.5.0 crate.New Rows and Sections
Metadata & Supporting Updates
decisions/key-rotation.mdto reference theNotImplementedstub.file-backend.jsonCI verification inspec/wire-format.md.Purpose
The core intent is trust accuracy: several cells previously claimed support for features that either had no backing code or could not be installed by users. The refresh ensures every cell reflects verifiable, shipped behavior with per-cell code evidence.
Summary
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single code-verified documentation refresh (LAB-1400). The changes are primarily documentation corrections tosdk-feature-matrix.mdand a clarification tospec/wire-format.md.Changes
sdk-feature-matrix.md2026-08-04to2026-08-03.<!-- -->HTML comment separators between adjacent GitHub alert blocks (> [!IMPORTANT]/> [!WARNING]) in two locations. This prevents consecutive callouts from being merged into a single block, ensuring they render as distinct alerts.mainonly and is absent from the published 0.5.0 crate, cross-referencing note ¹³. This aligns with the PR's stated intent that a "✅ a user cannot install" is treated as a trust bug.spec/wire-format.md*_binvectors.Purpose
The overarching goal is to correct the feature matrix so that documented feature status matches what code actually supports and what users can actually install, resolving inconsistencies introduced across ten separate open PRs. The changes emphasize distinguishing between features present on
mainversus features available in published releases.Summary
This PR consolidates ten conflicting
sdk-feature-matrix.mdpull requests (protocol#25, #28, #29, #31, #32, #33, #35, #37, #40, #43) into a single, code-verified refresh under LAB-1400. Critically, it corrects a class of trust bug where matrix cells describe a repository branch while claiming to describe a shipped SDK — a failure that recurred four times in six weeks and was reintroduced by the very audit meant to fix it.What Changed
Verification method (new decision record)
decisions/matrix-version-verification.md, establishing that version- and ship-status claims must be verified against the published artifact (registry metadata plus.crate/.tgzcontents when an embedded dependency decides the claim), not against a repo branch.X+), and that a verification date must accompany each claim.Corrected false cells
cachekit-rs0.6.0 published (withreliabilitydefault-on) 74 minutes before the prior revision's final commit, so the "unreleased" qualifier outlived its truth.binflip: The prior claim thatcachekit-ts0.1.5 ships the protocol-1.1binencoding is corrected to ❌ on both paths. The published 0.1.5 pins core-ts0.1.2(embeds core 0.2.0) and core-wasm0.1.1(core 0.3.0), verified by inspecting the shipped binaries.New matrix content
binflip.secure-API enforcement, documenting that TypeScript'scache.secure.wrap()is an unconditional alias forwrap()and that its degradation layer silently swallows decrypt failures (LAB-513, CWE-311).0.17.1+,0.6.0+, etc.).Footnote repair
¹–⁴defined twice with unrelated content); the Protocol Compliance block is renumbered to¹⁴–¹⁷so every marker resolves uniquely.CI enforcement
tools/check-version-floors.pyand wires it intoverify.yml. The check fails if any SDK Overview version is written as a bare snapshot (0.6.0) rather than a floor (0.6.0+). It catches snapshots masquerading as fact but not wrong floors — verifying floor accuracy remains reviewer discipline.Spec alignment
spec/wire-format.mdimplementation status to reflect that TypeScript emits legacy-only on both paths, with per-artifact evidence.Notes
Summary
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single code-verified documentation refresh, focusing on clarifying the "version floor" rule and its scope.Changes
Clarified the scope of the version floor rule (
decisions/matrix-version-verification.md):cachekit-core-0.2.0or a caret-free npm pin0.1.2), which should remain bare as factual evidence. It also notes that the CI guard polices only the SDK Overview table.Updated the matrix documentation note (
sdk-feature-matrix.md):+.Minor code cleanup (
tools/check-version-floors.py):ltorawinoverview_rows()for readability (avoiding the ambiguous single-letterl). No functional change.Purpose
The changes resolve ambiguity in the previous phrasing that could lead contributors to incorrectly append
+to exact artifact versions. By narrowing the floor rule to the SDK Overview table (which the CI guard enforces) and documenting the intent behind bare versions elsewhere, the PR prevents accidental "corrections" that would misrepresent artifact-specific version facts.Summary
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single code-verified documentation refresh, clarifying and refining the versioning convention that governs the SDK feature matrix.Key Changes
Refined the floor-vs-snapshot rule (
decisions/matrix-version-verification.md)X+), while a version that is evidence about one specific artifact stays bare.Updated the matrix guidance note (
sdk-feature-matrix.md)X+) and bare exact versions appear deliberately throughout the document, spelling out where each form applies.Updated the CI checker documentation (
tools/check-version-floors.py)Purpose
The changes tighten and clarify the versioning convention to prevent a recurring staleness problem (which the document notes has occurred four times), while resolving the conflict between multiple competing PRs by grounding the guidance in what can be code-verified against the registry.
Summary
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single code-verified refresh, correcting factual errors introduced by prior audit passes and hardening the CI guard that was silently overclaiming its coverage.What Changed
Corrected a false migration-risk claim (the core factual fix)
The previous documentation claimed that a reader built against
cachekit-core ≤ 0.3.0rejectsbin-encoded wire data, and instructed operators to sequence the TypeScript core republish ahead of any fleet upgrade. The code changes correct this acrosssdk-feature-matrix.mdandspec/wire-format.md:binwire, and a 1.1 reader accepts legacy wire. Thebinflip is therefore not a breaking change and needs no rollout ordering.cachekit-core/tests/dual_decode.rs) rather than a traced code mechanism.Rewrote the TypeScript encryption failure-mode documentation
Footnote ⁸ is expanded to clarify that TypeScript's reliability layer silently absorbs all encryption failures on both reads and writes (not just reads):
set()resolve like success while storing nothing.NonceExhaustedErroris raised inside thesetcallback and absorbed, so the caller cannot observe it. The Key-rotation row is updated to record this detection as unobservable rather than a safeguard.Documented that
--features workersdoes not compile (new footnote ¹⁹)Added a note explaining that
cargo add cachekit-rs --features workersdoes not compile because Cargo features are additive and the default set includesl1/reliability, which havecompile_error!guards againstworkers. The correct invocation is--no-default-features --features workers. This footnote is referenced from the Workers backend, TTL/locking, and WASM rows.Hardened the CI version-floor guard
tools/check-version-floors.pywas rewritten to fail closed: it now errors (rather than silently passing) if the SDK Overview table can't be located or parsed, dynamically locates the version column by header name, and correctly handles decorated cells (backticks, bold) and various placeholder dash characters.tools/test_check_version_floors.py(new) adds a 15-case mutation suite that runs first in CI, so the guard cannot silently degrade to reporting OK — motivated by the guard's own first version, which passed a snapshot hidden behind an ASCII-hyphen and rejected a valid backticked floor.Corrected the decision record's overclaiming
decisions/matrix-version-verification.mdandverify.ymlpreviously claimed the CI guard encoded "the one failure mode that recurred four times." The updated record is honest that the guard catches only one of six documented incidents — the rest (a ✅ on dead code, a ship-status boolean, two footnote regressions) require a reader, not a regex — because an overclaiming gate converts a known gap into an assumed-safe one.Why
Four of six matrix failures were committed by the audits meant to fix the earlier two, each written by someone applying a correct general rule to a claim they hadn't opened the artifact for. This refresh adds rule 6 to the decision record — for behavioural claims, cite the executed test, not a traced mechanism — and aligns the docs, guard, and CI accordingly.
Summary
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single, code-verified documentation refresh (LAB-1400), and hardens the CI guard that partially enforces the matrix's version rules.Documentation Corrections (
sdk-feature-matrix.md)Rust encryption feature-flag trap (new note ¹⁹): Documents that
--no-default-features --features workersbuilds must explicitly includeencryption,cachekitio. Adds a security warning that dropping theencryptionfeature turnsCacheKitBuilder::encryption()/::encryption_from_bytes()into silent no-op stubs returningOk(self), causing the cache to store plaintext at rest with no error (CWE-311). The AES-256-GCM cell now carries a footnote pointing to this assumption.Corrects the nonce-exhaustion narrative (notes ⁵ and ⁸): Clarifies that nonce exhaustion does not make master-key rotation mandatory.
cachekit-corefails closed at 2³² operations (generate_nonce()returnsNonceCounterExhaustedoncecounter >= u32::MAX, backed by anAtomicU64so it stays exhausted). The real consequence is that every encryptedset()on that encryptor silently stores nothing; the correct remedy is a fresh encryptor instance, not a master-key rotation (which would destroy all existing entries).Refines the TypeScript fail-open description (note ⁸): Corrects the failure-mode cell to "fail-OPEN on reads, silently drops writes, not switchable" and clarifies that L1 hits short-circuit before the reliability executor (running no crypto) and
acquireLocksits outside it.Clarifies Python's fail-open path (note ¹⁸): Documents that the same
encryption_fail_closedsetting drives both the fingerprint pre-check and the AES-GCM authentication-failure policy viahandle_decrypt_failure, both defaulting to recompute.CI Guard Hardening (
tools/check-version-floors.py)Footnote markers now permitted in version cells: The
DECORATIONregex accepts Unicode superscript characters (⁰¹²³⁴–⁹) so a floor carrying a footnote marker is not rejected.Second-table detection: The Overview table scanner now keeps scanning after the first table ends and fails loudly if a second table appears under the same heading, rather than silently ignoring content it cannot verify — closing a gap where a snapshot could hide in a second table.
Test & Documentation Honesty
Mutation suite grows from 15 to 17 cases (
test_check_version_floors.py), adding coverage for a floor carrying a footnote marker (must pass) and a snapshot hidden in a second table under the same heading (must fail).Honest accounting of the guard's reach (
decisions/matrix-version-verification.md,verify.yml): Comments and consequences are rewritten to state the guard catches only one facet of one of six matrix incidents, explicitly listing the five it cannot see. This avoids the overclaiming that a prior revision made ("the one failure mode that recurred four times"), which would falsely signal to auditors that CI has the matrix covered.Description
This PR consolidates ten conflicting
sdk-feature-matrixPRs into a single code-verified refresh, addressing accuracy and clarity issues in the version verification tooling.Changes
Clarified tool scope documentation (
tools/check-version-floors.py)cachekit-rs0.5.0/0.6.0 version snapshot mismatch), and explicitly notes it does not catch the six mis-marked Reliability cells from that same event nor the other five incidents.Made the placeholder regex robust to visually ambiguous characters
\u2014,\u2013,\u002d) since these three characters are visually indistinguishable in source. Added a comment explaining the intent and confirming no version string can hide inside a placeholder cell.Test alignment (
tools/test_check_version_floors.py)Consistent messaging
—) with semicolons for consistency across both files.Purpose
The changes improve the reliability and readability of the version-floor checker by eliminating ambiguity around visually similar dash characters, and honestly document the tool's narrow detection scope so its limitations are not misunderstood.
Summary by CodeRabbit
Documentation
Tests