Repository navigation
Conversation
A release build with the deterministic-kms feature now fails to compile. That feature exports _use_deterministic_keys. That function sets every key in the process from a fixed seed. Only the dev-profile test build uses it. The wheel import step also fails if a wheel exports it. tests-python.yml: - The live job skips Dependabot's pull requests. They get no Actions secrets, so require-cs-secrets failed on every one. - An example-only change no longer starts the workflow. CI does not run the example's tests. - .cargo/** and EQL's Cargo.toml and Cargo.lock now start it. Cargo reads the first for this workspace. The EQL build uses the other two. - A pull request's wheel records the PR's head commit. GitHub's test merge commit is on no branch, so a bug report could not name it. - maturin is fixed at v1.16.0, inside pyproject.toml's range. - wheels-summary runs when a leg fails, so the passing legs still get their build-time row. Addresses review threads 1513-F5, 1499-F2, 1499-F12, 1621-F5, and 1406-F1, F2, F3, F5. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The decorator replaced a hand-written __repr__ or __str__ with no error in four cases: a method wrapped by reprlib.recursive_repr, a staticmethod, a partialmethod, or a function from another module. Now the decorator replaces only a method that dataclasses, attrs, or NamedTuple generated. It refuses anything else on the class with declaration_repr_present. The field reader had five faults: - It refused a correctly placed marker around a generic value type, such as re.Pattern[str]. - It read dunder annotations, such as __tablename__, as fields. - An AttributeError or a SyntaxError from a hint escaped with no field name. - Before Python 3.14, it named the wrong field after a string ClassVar hint. - It did not refuse a required Ignore field with a Pydantic alias or an attrs private name. The fixes: - Only a SQLAlchemy wrapper is refused inside the marker's Annotated. - Dunder names are skipped. - The four hint errors give declaration_hints_unresolved. - Each hint is resolved on a probe class, not a probe function. - An Ignore field is looked up by its constructor name. - A Pydantic dataclass's config is read from __pydantic_config__. Addresses review threads 1453-F1, F2, 1433-F1, F2, F3, F5, and 1437-F1, F2, F3. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
encrypt read each field by name, so a model of another class with the same field names was encrypted under this table. It now refuses a model that is not an instance of the declared class, with value_wrong_type. A datetime whose tzinfo gives no UTC offset is naive in Python. The value reader now checks utcoffset(), so such a value gives value_naive_datetime, not a TypeError. decrypt turned every error from json.dumps into decrypt_malformed. That included KeyboardInterrupt and MemoryError. Now only TypeError, ValueError, and RecursionError do so. An error raised while a passthrough value is read now propagates, as it does on encrypt. One row passed on its own, for example from fetchone(), was read as a list of column names. decrypt, decrypt_rows, and encrypt_rows now refuse it and say to pass [row]. A str with a lone surrogate now gives decrypt_malformed, the same as bytes that are not UTF-8. encrypt now has the type Sequence[T]. Addresses review threads 1499-F5, 1499-F6, and 1509-F4, F5, F6, F8, F9. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewers marked ten paths as having no test. Each one could break with no test failing. - The engine commit is checked against git: the SHA must equal HEAD, with -dirty exactly when a stack-* crate has changed outside fuzz/. build.rs now records where the commit came from. cargo sets the commit variable when it runs the tests, so the test cannot read the environment to find an override. - The frame reader walks a typed null, a sequence, and a nested map. A frame writer writes a typed null as the plain null, so the test expects no type there. - The SIGINT gate tests cancel and join their timer in finally. - A class read from its hints leaves out ClassVar and InitVar, and rule A holds inside Mapped on a real mapped class. - repr of an expired or detached SQLAlchemy row sends no SQL statement. - After the switch to the deterministic client, the live client and a second switch are refused. test:cargo runs this test binary with the deterministic-kms feature. A seed that is not 32 bytes is refused. - decrypt runs from 8 threads at once, live and deterministic. - The keyset test fails if the load-keyset counter is missing. Addresses review threads 1405-F2, 1414-F4, F6, 1433-F6, F7, 1453-F7, 1513-F2, F3, and 1516-F1, F2. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each change is small. Together they close the review threads tagged as quick wins. Build and engine: - build.rs fails when git status fails. Before, it recorded a clean commit. -dirty now also covers the root Cargo.toml and the Python Cargo.lock, which choose the engine's dependency versions. - run! gives its value in both builds. Before, the test build returned from the calling function. - The allocator spy uses a poisoned lock as it is. A panic inside the allocator would abort the test binary with no message. Tests: - The value tests refuse both sides of each integer range, a float too large for float32, and a decimal mantissa one past 96 bits. The edge-value test builds the cipher outside pytest.raises. - Refusals of a hand-written or replaced __str__ and __rich_repr__, and of a changed or removed hint. - A constructor's own error reaches the caller of decrypt unchanged. A dict with no JSON form gives decrypt_malformed. - The thread helpers time out and name the failed thread. Other helpers give clearer failure messages, and the Go-row tests check lengths before zip. - The conformance test compares vector names, not counts. - The CI script tests check that their scans find something, test the triggers() helper, and hold the deterministic build's features to pyproject.toml's. - The codegen test shows that a refused override writes no file. - The ClassVar test from the last commit keeps its bare ClassVar case for Python 3.10 and later. Python 3.9 cannot resolve one written as a string. Docs and example: - AGENTS.md says which job builds the wheels, where extension-module is named, and what test:cargo runs. - G6 on the scorecard is strained: it lists two gaps with no test. - The example builds EQL before it deletes its database volume, says to trust EQL's mise file, and accepts a postgres:// URL. Addresses review threads 1405-F1, F4, 1406-F6, F7, F8, F9, 1414-F3, F5, 1415-F4, 1453-F8, F9, 1499-F17, 1509-F10, F11, 1511-F1, F3, F4, F5, 1513-F1, F4, F7, F8, 1516-F3, 1517-F1, F2, F7, F8, and 1621-F1, F2. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under from __future__ import annotations, Python 3.9 cannot read a bare ClassVar hint: get_type_hints raises TypeError. The decorator reported it as an unresolved name, and told the user to define or import it. Nothing was missing, so that advice did not help. The decorator now evaluates the hint that failed. If it is ClassVar with no type, the error says to write ClassVar[int], with the reason bare_class_var. The evaluation is Python's own, so typing.ClassVar and an alias are found too. Python versions that read the bare form accept it as before. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The engine client includes a tokio runtime with its own threads. fork() copies only the thread that called it, so a child that Gunicorn or multiprocessing forked after the first call held a client whose runtime had no threads. Its next call never returned. The client is now stored with the ID of the process that built it. A child builds its own client on its first call, and loads the keyset again. The parent's copy is never dropped in the child: dropping a runtime whose threads are gone could block. macOS does not allow its TLS certificate service in a child that fork made, so on macOS the child crashes when it builds its client. The test skips on macOS, and test:live leaves it out there. CI runs it on Linux. run! also had a label that only the test build used. clippy with -D warnings refused it in the normal build. It is allowed now. Addresses review thread 1499-F4. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The engine client includes a tokio runtime with its own threads. fork() copies only the thread that called it, so a child forked after the first call held a client whose runtime had no threads. Its next call never returned. Building a new runtime in the child is not safe either. tokio's locks keep a process-wide table of waiting threads, and the child's copy lists the parent's threads. On Linux, a new runtime in the child blocked in 8 of 15 runs. So a child that fork made after the client was built is now refused, with StashError code stack_forked. The message says to make the first call after the fork. The check reads a OnceLock and an atomic process ID, and takes no lock, so it cannot block. A child forked before any call builds its own client, as before. The fork test covers both cases, and runs on macOS and Linux. On Linux, each case passed 15 of 15 runs. Addresses review thread 1499-F4. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…stays so AGENTS.md said the Python Cargo.lock is seeded from the root one, so the crates the two workspaces share resolve to the versions the root tests. That was no longer true: the Python lock had vitaminc 0.8.1, and the root lock had 0.8.0. No check compared the two locks. The Python lock is seeded from the root lock again. The difference was small: 0.8.1 adds a storage profile to vitaminc-aead-frame. The SDK does not use it. The encryption and order crates have the same source in both versions. The documented steps were not enough to re-seed: cargo metadata moved vitaminc back to 0.8.1, and cargo update --precise moved each crate to 0.8.0. AGENTS.md now gives that step. python-lock-parity.test.mjs compares the crates that decide what a stored value is, by version, source and checksum: every vitaminc crate, recipher, ore-rs, clww-ore, cts-common and zerokms-protocol. Other shared crates may differ. The lock parser moves to lib/cargo-lock.mjs, so this test and the cipherstash-client one share it. The deterministic suite still opens the Go SDK's frozen rows and seals the same values, so vitaminc 0.8.0 gives the same results here. Addresses review thread 1406-F4. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The live job set the four CS_* credentials for the whole job. So every step could read them, the builds included. A build runs third-party code: each dependency's build.rs and proc-macros, and maturin. The credentials now go to the Live tests step only. A step before it builds the extension and the EQL bundle, with no credentials. The live tests run through test:live:run, a new task that builds nothing. build:eql-bundle is the other new task. test:live still builds and runs everything, for local use. python-ci.test.mjs requires CI to run every Python task. test:live is exempt, with its reason, because CI runs it in parts. maturin is still installed unlocked (review thread 1582-F1). It now runs without the credentials. Addresses review thread 1499-F1, and in part 1582-F1. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The value_frame target read its input as an arbitrary Input enum, and the committed seeds are plain value frames. arbitrary reads the first four bytes as the enum tag. So six seeds decoded as Write, and the seventh as a Read of one byte. No seed reached the read check, and CI's replay never ran it. Putting the Read tag in front of each seed is not enough. arbitrary 1.5.0 puts a byte of its own before each Vec<u8> element, so an arbitrary input never contains a frame byte for byte. The target now takes raw bytes, and the first byte chooses the branch. 0x00 is Read, and the rest of the input is the frame, byte for byte. Any other byte is Write, decoded through arbitrary as before. Each seed now starts with 0x00. On current nightly, all seven seeds reach the read check, and the replay passes. Addresses review thread 1511-F2. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The example found gaps in the SDK and its documentation while it was built. This file records them, with the timings it measured and the README claims that held. Items 5, 6 and 7 were found before the SQLAlchemy layer existed, and the layer addresses each one. Each now has a status line that says how. Item 8 is new: when the engine refuses a value, decrypt_malformed and decrypt_kind_mismatch name the field but not the row. The Go SDK has the same gap. The suggested fix is in the engine, so that both SDKs get the row (review thread 1509-F7). Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Most decrypt errors name the row and the field. decrypt_malformed and decrypt_kind_mismatch from the engine name the field only, because the engine does not report the row. The README and the Cipher.decrypt docstring now say so, and say where to look for the bad value. The fix belongs in the engine, so that the Python and Go SDKs both get the row. FINDINGS.md item 8 records it. Addresses review thread 1509-F7. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…own code The Python SDK builds its engine client over a connection that counts its requests, so it cannot use StackCipherBuilder. It copied two private pieces of that builder: the reader of CS_ZEROKMS_HOST in stack-kms, and the client key provider in stack-encrypt. A change to either would not reach Python, and no test would report it. stack-kms now exports endpoint_from_env, the reader the builder uses. stack-encrypt now exports client_key_provider, the provider init uses: the environment first, then the stash profile. The Python client calls both, and its copies are gone. The two copies had already drifted: with no client key and no profile, Python now gives the engine's message, which says npx stash auth login. The wasm32 variant of client_key_provider named KeyProvider, which is imported only for other targets. It now uses the full path. Nothing in CI builds stack-encrypt's http feature for wasm32. Addresses review thread 1499-F15. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
uv.lock fixed the test tools, but not maturin, the build backend. Each build downloaded maturin from PyPI at any version in [build-system]'s range, with no hash check and no Dependabot cooldown. maturin is now in a build dependency group, so uv.lock records its version and hash. Dependabot updates it with the test extra. mise run build installs the group into .venv, and builds the package with --no-build-isolation, so uv downloads no maturin of its own. Without maturin in .venv, that build fails. [build-system] keeps its range, for an install by any other route. The wheels job still uses maturin-action, with maturin's version fixed in the workflow. Addresses review thread 1582-F1. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Mutation testing (cargo-mutants,
|
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Reviewed 75c78105 · posted from Claude Code (scheduled routine) · claude-opus-5-5 · effort medium
Sources: code-review · claude-opus-5-5
Skills: code-review, rust-security, review-cross-check
Findings (9): 0 critical, 3 major, 6 minor. All 9 are inline, 0 below.
Every finding was checked against 75c78105 by separate verification agents. Most were reproduced, and the fork hang and attrs __str__ findings among them. One finding was dropped as refuted: the not(debug_assertions) guard on deterministic-kms (crates/core/src/lib.rs:32) is reachable only through a build that names the feature on purpose. The rust-security pass over the changed Rust found nothing new. That pass covered endpoint_from_env, client_key_provider, the decrypt error paths and the deterministic-kms gating. The PR had no earlier review findings to cross-check against.
Generated by Claude Code
| if let Some(client) = current()? { | ||
| return Ok(client); | ||
| } | ||
| let _building = locked(&BUILDING); |
There was a problem hiding this comment.
major The fork refusal only fires once CLIENT is set. If a child is forked while another thread holds BUILDING, from this line until shared() returns (including the network keyset load in build()), then the child's first call gets Ok(None) from current() and blocks here forever, on a mutex whose owner does not exist in the child. A trigger is Gunicorn --preload or multiprocessing fork while a warm-up thread is making the first call. This was reproduced with a copy of this lock order: the child was still blocked 6s after the parent's build finished. One fix is to store the builder's PID in an AtomicU32 right after taking BUILDING, have waiters loop on try_lock, and refuse with stack_forked when that PID is set and is not std::process::id(). use_deterministic (line 200) uses the same lock pattern.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| """``rows`` as a list. Raises ``error`` with ``code`` for one row on its | ||
| own, such as ``cursor.fetchone()`` returns: ``list()`` of a mapping is its | ||
| keys, and each key would be refused as a row that is not a mapping.""" | ||
| if isinstance(rows, Mapping): |
There was a problem hiding this comment.
major Only a Mapping is refused as a lone row. A SQLAlchemy Row or a psycopg tuple row is a sequence, so list(rows) splits it into its column values. When every selected column is sealed, each value is an EQL dict. _row_fields then finds no declared field and decrypt_rows returns the ciphertext dicts unchanged, with no error. This was reproduced: decrypt_rows(row) with row = ({"c": …}, {"c": …}) returns the two payloads as they were. Fix: also refuse an object with _mapping (point to row._mapping or .mappings()). encrypt(models) at line 98 needs a guard too: refuse isinstance(models, self._cls) before list(), because a Pydantic model or a NamedTuple model is iterated into its fields.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| return True | ||
| if filename == _DATACLASS_FILE: | ||
| return inner.__qualname__.startswith(_DATACLASS_QUALNAME) | ||
| return _COLLECTIONS_FILE is not None and os.path.realpath(filename) == _COLLECTIONS_FILE |
There was a problem hiding this comment.
major @attrs.define(str=True) is now refused with declaration_repr_present. attrs builds that __str__ as a plain closure in attr/_make.py, not under <attrs generated, so none of the three cases here matches it. This was reproduced with attrs 26.1.0. The only way past it is redact=False, which turns redaction off. Fix: also accept code whose file is sys.modules["attr._make"].__file__ and whose co_name is "__str__", and add a str=True test case.
Source: code-review · claude-opus-5-5
Generated by Claude Code
|
|
||
| # Builds the wheel and the bundle, installs the bundle, and runs the | ||
| # live tests; a skip fails the task. | ||
| # The builds, with no credentials: the extension into .venv, and the |
There was a problem hiding this comment.
minor The build step and the Live tests step run on the same runner and filesystem. Anything the build step leaves behind therefore runs with CS_CLIENT_KEY and CS_CLIENT_ACCESS_KEY set: a .pth in .venv, a $GITHUB_ENV or $GITHUB_PATH line, an edited mise.toml, or a changed shim. Dependency code compiled into _engine.so does too. This is still better than the job-level secrets it replaces, but this comment, mise.toml:108-111 and the AGENTS.md line ("no dependency's build script runs with the credentials set") claim more than the change delivers. Either reword them, or build the wheel in a job with no secrets and install only that wheel into a fresh venv in the live job.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| id: engine | ||
| run: echo "commit=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" | ||
| env: | ||
| COMMIT: ${{ github.event.pull_request.head.sha || github.sha }} |
There was a problem hiding this comment.
minor The wheels checkout (line 387) has no ref:, so on a pull request it compiles refs/pull/N/merge, while this records head.sha. build.rs trusts the override and skips the -dirty check. So when the base branch has moved under packages/stack-*, __engine__ names a commit the wheel was not built from. Fix: add ref: ${{ github.event.pull_request.head.sha || github.sha }} to that checkout.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| rust-toolchain: ${{ steps.rust.outputs.version }} | ||
| # A fixed maturin, inside pyproject.toml's range. Without it the | ||
| # action downloads the newest release on every run. | ||
| maturin-version: v1.16.0 |
There was a problem hiding this comment.
minor Nothing keeps this pin equal to maturin in uv.lock (1.16.0). Dependabot's uv group will bump the lock alone, and then mise run build and the live job build with the new maturin while the shipped wheels stay on 1.16.0, with no failure. Add an assertion in scripts/__tests__/python-ci.test.mjs that this value, without the v, equals the uv.lock version. The dependabot.yml comment that says uv.lock "holds only the test extra" is now stale as well.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| 'pyproject.toml has a [tool.maturin] features list', | ||
| ).not.toBeNull() | ||
| const normal = [...declared[1].matchAll(/"([^"]+)"/g)].map((m) => m[1]) | ||
| const task = read(`${PYTHON_ROOT}/mise.toml`).match(/--features ([^\s"]+)/) |
There was a problem hiding this comment.
minor This takes the first --features match anywhere in mise.toml. This PR adds a second one (test:cargo, line 90), so the check now depends on block order. Moving test:cargo above test:deterministic makes it capture deterministic-kms alone and fail on a correct file. This was reproduced. Slice the [tasks."test:deterministic"] body first, as the localOnly check does.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| /// (`int8`, the unsigned and 128-bit integers, `uuid`), it is not a scalar | ||
| /// (`array`, `object`), or it is a timestamp in a unit other than the | ||
| /// nanoseconds the SDK writes. | ||
| /// The vectors the SDK writes, in corpus order: every scalar kind it has, |
There was a problem hiding this comment.
minor WRITTEN was inserted between NOT_WRITTEN's doc comment and NOT_WRITTEN, so all seven /// lines now document WRITTEN. It is described as the vectors "the SDK never writes", and NOT_WRITTEN (line 49) has no doc. Move lines 20-23 down to just above const NOT_WRITTEN.
Source: code-review · claude-opus-5-5
Generated by Claude Code
| a ``ClassVar``, and before 3.14 a string ``ClassVar`` hint raises there. | ||
| """ | ||
| module = sys.modules.get(base.__module__) | ||
| global_names = dict(vars(module)) if module is not None else {} |
There was a problem hiding this comment.
minor dict(vars(module)) and dict(vars(base)) are now rebuilt for every hint, not once per base. This runs on the relationship path of _hints, at decoration and at each Cipher(...), and again in _unresolved_field and _is_bare_class_var. Compute global_names and base_locals once per base and pass them in.
Source: code-review · claude-opus-5-5
Generated by Claude Code
The comment on the uv entry said uv.lock contains only the test extra. It now also locks maturin in the build dependency group. Dependabot's uv parser reads [dependency-groups] (uv/helpers/lib/parser.py in dependabot-core), so maturin gets the same updates and cooldown. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t hooks - Pydantic builds __repr_str__ and devtools' __pretty__ from __repr_args__, which still showed plaintext. It is now replaced on a Pydantic model with the sealed fields. - Refusal::of joined an engine message and its help with a space, so the two read as one broken sentence. A full stop now ends the message. - NotAuthenticated is also raised for a profile that lacks its region or client ID. Its message no longer says that nothing was found. - Core's query_unsupported now names the marker in reason, as the Python check does. Addresses review threads 1453-F3, F4, F5, and 1510-F1. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tor guards - A date and a datetime read as known engine numbers, before the epoch and with a non-UTC offset, and back. A decimal reads as its exact mantissa and scale, signs and positive exponents included. - The STACKENCRYPT_ENGINE_COMMIT rules moved from build.rs into engine_commit_rules.rs, which build.rs and a unit test both include. - The frame reader reads a batch written the way dynamic::decrypt writes one. - The _eql.py generator's guards have tests. A new py_ident check refuses a catalog name that is not a usable Python class name. Addresses review threads 1405-F3, 1414-F5, 1415-F2, F3, and 1499-F13, F14. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- An offline test is skipped once a live test has built the shared engine client in the same process. A combined run had 51 failures. - refused() checks that the refused value is not in the error text. - The same value encrypts to three different ciphertexts. - The declaration cases are compiled with pytest's assertion rewrite, so a failure shows its values. - New tests for four README claims: a redact=False class may replace its printing methods, decrypt refuses a uuid.UUID for a str field, and bound_names keeps only top-level names, now with except-as and a walrus in a comprehension. - The gate test says that CI does not run it, and what its timing assertion can show. Addresses review threads 1414-F1, F2, 1433-F4, 1499-F8, F10, F11, and 1517-F3, F4, F5, F6. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The live job had one rust-cache step per workspace. Each pruned the shared cargo registry to its own crates before it saved, so each save lost the other's. One step now lists both. eql-suite-ci.test.mjs reads workspaces: as a list. python-ci.test.mjs now checks that the workflow runs the _eql.py parity test, and that the build task installs from uv.lock with --locked, --no-deps and --no-build-isolation. Addresses review threads 1415-F1, 1499-F16, and 1582-F2. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
yujiyokoo
changed the title
The customer directory example recorded four gaps in the README: - Pydantic's ValidationError and FastAPI's 422 response show the rejected input. The README's list of what shows plaintext now names both. hide_input_in_errors hides the input from the error's text, but not from errors(), which FastAPI's response is built from. - A key the database generates cannot be a required field. The UUID key section now covers it, for psycopg and for the SQLAlchemy layer. - decrypt keeps the order of the rows. The README and the docstring now say so, and a deterministic test checks it with the rows reversed. - A unique index on eq_term works because equal values have equal eq terms. The README now says so. Each finding in FINDINGS.md now has a status line. Refs #1341 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Reviewers left 113 review threads across the 17 pull requests of the Python SDK stack. This pull request fixes the ones that were still open, on top of #1622, including most of the smaller ones tagged "low". The fixes cover wrong error codes and declaration checks, a hang in a forked worker process, CI secrets that build code could read, and tests that could pass while checking nothing.
Changes
Declarations
__repr__,__str__or__rich_repr__in any form, not only a plain function.ClassVar, and a requiredIgnorefield with an alias or a private name.ClassVarwritten as a string now gets an error that says how to fix it.Redaction and messages
__repr_args__also shows<sealed>. Pydantic's__repr_str__and devtools'debug()print through it.query_unsupportednames the marker inreason, as the Python check does.Encrypt and decrypt
encryptrefuses a model of another class. A datetime with no UTC offset is naive.decryptkeepsKeyboardInterruptandMemoryError, refuses one row passed on its own, and refuses astrthat is not valid UTF-8.Cipher.decryptdocstring say which two engine errors do not name the row.Processes
stack_forked. Before, its next call never returned. A new runtime in the child is not safe either: on Linux it blocked in 8 of 15 runs.Engine crates
stack-kmsexportsendpoint_from_env, andstack-encryptexportsclient_key_provider. The Python client calls them, in place of its own copies.Build and CI
deterministic-kmstest feature fails to compile, and the wheel check refuses a wheel that has it.livejob gives theCS_*credentials to the test step only. The build runs before it, without them.uv.lock, andmise run builduses that copy.Cargo.lockresolves the root workspace'svitamincagain. A new test compares the crates that decide stored values in the two locks.livejob caches both Cargo workspaces in onerust-cachestep. Two steps deleted each other's cached crates._eql.pyparity test, and that the build installs fromuv.lockwith no tool from outside it.Tests
_eql.pygenerator's guards, have tests.Docs
FINDINGS.md. Each of its 8 items has a status line.eq_termworks.Cipher.decryptdocstring say thatdecryptkeeps the order of the rows. A deterministic test checks it.Verification
Run locally on macOS, on the last commit unless stated:
mise run test:liveagainst ZeroKMS and PostgreSQL 17: 98 passed.mise run test:deterministic: 24 passed.mise run test:cargo, clippy with and without features, andcargo fmt --checkpassed.stack-kmsandstack-encrypt: nextest 1128 passed, rustdoc with warnings as errors passed.Run on Linux in Docker:
CI passed at
bf84840af, the last commit: 63 checks passed. 2 were skipped,fuzz campaignandreview, which do not run on a pull request. Thelivejob ran the fork test on Linux, and both cases passed.Not run:
gate-probebuild. CI builds no such wheel.Related
Refs #1341
Review notes
c7d6eb1b7rebuilt the client in a forked child.3c2b14ea9replaces that with the refusal, after the Linux runs showed the rebuild can block. The two could be squashed.FINDINGS.mditem 8.🤖 Generated with Claude Code