Repository navigation
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 819 files, which is 719 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files.
 |
🦋 Changeset detectedLatest commit: ffeed7a The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 279 | 0 | 185 | 0 |
cargo-mutants exited with no surviving mutants recorded. The run may not have completed cleanly; check the workflow run logs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a05affaaa5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
freshtonic
previously requested changes
Oct 23, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
I request changes for two release problems on the crates.io side. The code itself is in good shape, and every check I ran passes.
How I reviewed this
The diff is large because generated code is committed. I reviewed the generators and their templates, then I ran the drift checks to prove that the committed output matches them. I did not read each generated file.
- EQL codegen:
eql-codegenrenders the version-neutral SQL source two times (eql_v3andeql_v4) from theeql-domainscatalogue. It also generates the Rust payload types andeql_gen.go. - Go:
stashgengenerates the*_stash.gofiles and the goldens.
Checks run (in a worktree at a05affaaa)
- EQL:
cargo test -p eql-codegen(includes the Go parity test): pass.mise run types:check,codegen:parity,test:schemas:parity: pass.mise run build, with the installer completeness gate and the v4 isolation gate: pass.git statuswas clean after all of these.
- Rust and Go:
cargo fmt --check: pass.cargo clippy --workspace --all-features --all-targets -D warnings: pass.cargo test -p stack-guest-abi -p stack-encrypt --all-features: pass.
- Go guests and SDK:
- All five WASI guests build, and the import-surface checks pass.
go vet ./...andgo test ./...inlanguages/golang: pass.go generate ./...leaves no diff.
- I built the v3 bundle and compared it with the
@cipherstash/eql@3.0.6tarball. Onlylints()and comments changed, so the claim "v3 SQL unchanged" holds. git merge-treeagainst currentmain: no conflicts.cargo publish --dry-run -p stack-encrypt-derive -p stack-encryptagainst crates.io: fails (see below).
Blocking
- stack-encrypt 0.3.0 cannot publish. The root release-plz line publishes every stack-* version that crates.io does not have, so merging this starts a publish of stack-encrypt 0.3.0. stack-encrypt requires
stack-kms0.1.0, and that is the only stack-kms version on crates.io. Butsrc/lib.rsre-exportsstack_kms::{diagnostic, ErrorPayload}, and these were added after the 0.1.0 release with no version bump. The packaged build fails withE0432: unresolved imports stack_kms::diagnostic, stack_kms::ErrorPayload.- Fix: bump stack-kms (for example to 0.2.0, with a CHANGELOG entry), and make stack-encrypt require that version.
- Note that the publish dry run on
main(tests-crates.yml) packages all five crates together, so stack-encrypt resolves the tree's stack-kms there. That step does not catch a stack-kms that crates.io lacks under the same version number.
- The stack-encrypt CHANGELOG has no 0.3.0 section.
Cargo.tomlsays 0.3.0, but about 850 lines, including a### Breakingsection, are still under## [Unreleased]. The root release-plz line only publishes and never rewrites the changelog, so 0.3.0 would ship under "Unreleased", as 0.1.0 did. Please rename the section to## [0.3.0] - <date>in this PR.
Non-blocking (see inline)
SchemaVersionis now generic. Rust does not apply a default const parameter in an expression, soSchemaVersion::CURRENTandSchemaVersion::default()stop compiling (E0284). The changeset ships this as a minor bump and says "plainSchemaVersionstill means v3".- The stored and single-bound query domain templates accept
"v": null. The range template already guards against this. - stack-encrypt will publish with an exact pin on
cllw-ore =0.5.2-rc.1. - There are some small nits: stale vitaminc version numbers in comments, a Go wipe path, an ADR reference, and the position of a
use.
Before you merge
eql-bindings now requires stack-encrypt 0.3.0. Its crates.io publish (publish_all_features) succeeds only after release-crates has published stack-encrypt 0.3.0, so blocker 1 must be fixed first. Otherwise the EQL crate publish fails too.
Checked and fine
- SQL renderer: it refuses literal
eql_v3/eql_v4names, unknown placeholders and unmatched blocks. - Range domains and
between: correct, with STRICT/IMMUTABLE accessors and an inlinablebetween. On non-superuser installs, the ORE range domains are disabled with the same NOT VALID CHECK as the single-bound ORE domains. - v4
ore_block_v2SQL: the layout offsets and the GF(2^128) doubling are correct, and errors do not echo the input bytes. - stack-guest-abi
stepandvalue: no new raw reads of host lengths, handles are never reused,MAX_OPENis enforced, and secrets areZeroizingwithOpaqueDebug. - Go step loop: every early exit after a pause aborts the operation and wipes the buffers.
maxPausedis tied to the Rust constant by a test. - Go public surface:
QueryTerm,VerifyandTargetLimitfollow the existingSeal/Openpattern, andMatchQuerynever prints its text. - CI: actions are pinned by SHA, both copies of the path filters were updated, and no release workflow changed.
- Changesets: the EQL changesets are minor, which is correct apart from the
SchemaVersionpoint.chacha20-0-10-2.mdmatches the lockfile moves. - Skills: no skill update is needed.
stash eql installstill installs v3 only, and no skill covers the Go SDK or Stack Encrypt.
| @@ -1,7 +1,7 @@ | |||
| [package] | |||
| name = "stack-encrypt" | |||
| description = "Encrypt Rust values under per-value ZeroKMS data keys via the vitaminc cipher traits" | |||
| version = "0.2.0" | |||
| version = "0.3.0" | |||
Contributor
There was a problem hiding this comment.
Blocking. Merging this starts a crates.io publish of stack-encrypt 0.3.0. That publish fails. This crate requires stack-kms 0.1.0 (the only stack-kms on crates.io), but src/lib.rs re-exports stack_kms::{diagnostic, ErrorPayload}. These came after the 0.1.0 release, and the version was not bumped. cargo publish --dry-run -p stack-encrypt-derive -p stack-encrypt fails with E0432 at src/lib.rs:300.
Please bump stack-kms (for example to 0.2.0, with a CHANGELOG entry) and require that version here. Then release-plz publishes stack-kms first.
Also blocking: CHANGELOG.md still has these changes under ## [Unreleased]. The root release-plz line does not rewrite the changelog, so 0.3.0 would ship with an "Unreleased" heading. Please rename it to ## [0.3.0] - <date>.
Contributor
Author
There was a problem hiding this comment.
Fixed in #1403, commit 9590f83. The gap was wider than stack-kms. Run against crates.io on current next, cargo publish --dry-run -p stack-kms fails with stack_auth::diagnostic and stack_auth::tls unresolved, and -p stack-auth fails with stack_profile::diagnostic unresolved. Each crate changed after its 0.43.0 / 0.1.0 release with no version bump.
The commit:
- moves stack-auth and stack-profile to 0.44.0 (stack-auth's per-JWT token cache change is breaking) and stack-kms to 0.2.0, and makes stack-encrypt require stack-kms 0.2.0;
- adds a 0.44.0 CHANGELOG section for stack-auth and stack-profile, and a first CHANGELOG for stack-kms;
- renames stack-encrypt's
## [Unreleased]to## [0.3.0] - 2026-10-27; - updates the lockfiles that record these crates (root, both Go guests, EQL, stack-encrypt fuzz);
- adds an
@cipherstash/authpatch changeset, because that package is built from stack-auth.
cargo publish --dry-run -p stack-profile -p stack-auth -p stack-kms -p stack-encrypt-derive -p stack-encrypt now packages and verifies all five.
I'm leaving this open for @coderdan to confirm: this also publishes stack-auth and stack-profile 0.44.0, and the CHANGELOG dates assume the merge to main happens on 2026-10-27.
freshtonic
dismissed
their stale review
October 23, 2026 05:17
Not blocking. The points in this review are still open, but they need not stop the merge.
A containment query now sends its selector set in a random order of its own (#1379), so two derivations of one document, typed or as text, serialize differently. The test that holds a typed needle to the query its JSON text derives compared bytes; it now sorts every list of both queries, recursively, and compares those. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
The mutants gate found Tokens::zeroize replaceable with a no-op and no test failing: a query's tokens are plaintext held in Zeroizing<Tokens>, so a zeroize that skipped either kind would leave them in memory. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
From review of #1397. match_term split the tokens into two plain Vecs, so a failure deriving the n-grams freed the short words unwiped; they are now held in Zeroizing until the PRF takes them. The standard tokenizer and the short-word filter now share one word split, so "a short word is a standard token" holds by construction. Docs say length is counted after case folding and which characters do not end a word; the eql-bindings README and the Go README describe the v4 match format as it now is; a test checks the filter alone, not the filter and verify together. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
The mutants gate found the depth cap's comparison unpinned: refusing a path of MAX_DEPTH steps instead of more than MAX_DEPTH passed every test. A selector path of 127 steps, the deepest that can name a node, is now among the inputs the check accepts. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the base's test that pins the JSON path depth cap at a document's depth. No conflicts. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Brings in the base's test that pins the JSON path depth cap at a document's depth. No conflicts. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
vcffi and vcvalue move from d7de535c4, the merge of cipherstash/vitaminc#526 into a feature branch, to 06568ba93, the commit cipherstash/vitaminc#544 put on vitaminc main. The bindings' files are the same at both commits, so no Go behaviour changes; the pin now names a commit a vitaminc release will carry. The bindings still have no tag, so the release guard keeps refusing a final version of this module until one exists. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…profile Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
Carries the vitaminc Go bindings pin to 06568ba93 on vitaminc main. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
…ile-git chore(deps): take vitaminc's Go bindings from cipherstash/vitaminc#526 by git reference
feat(go): seal an opaque struct as a vitaminc storage blob
feat(stack-encrypt,eql): store a JSON document through a data plan as JsonSearch
feat(go): generate encrypted JSON document fields
Brings in the Go json_tree stack's first pull requests (#1369, #1370, #1372). The only conflicts were adjacent CHANGELOG entries in the Go module and stack-encrypt, resolved by keeping both. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
feat(stack-encrypt, golang): JSON operators and data-plan reads, with generated Go JSON document fields (#1374)
feat(go): encrypted JSON documents of Go types, with typed needles
Brings in the rest of the Go json_tree stack (#1373, #1374, #1378). The only conflicts were adjacent CHANGELOG entries in the Go module and stack-encrypt, resolved by keeping both. Claude-Session: https://claude.ai/code/session_01N7bCqa3ZVJMcEE99RzVqEu
feat(stack-encrypt): make two-letter words such as "Ng" searchable in the match index
…ation docs(go): typed JSON walkthrough and accurate EQL reference
This branch was successfully deployed
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
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 is the integration branch for a release candidate of EQL v4 and of Stack Encrypt's Rust and Go support. EQL is the SQL that lets PostgreSQL store and search encrypted values; EQL v4 is a second copy of it for values written by Stack Encrypt, the Rust encryption engine behind the Go SDK. The open pull requests for both are merged here first, so they can be tested together before any of them reaches
main.The release version is not decided yet.
Why a branch, not main
Some of this work depends on vitaminc and ore.rs code that is not yet on crates.io, so it pins those crates to a git commit.
stack-encryptandeql-bindingsare published crates, andcargo publishdrops those git pins, so nothing on this branch can be released until vitaminc 0.6 is published (cipherstash/vitaminc#389, which waits on cipherstash/vitaminc#463). Once it is, the git pins become crates.io versions and this branch merges tomainas a normal release.What lands here, in order
feat/aead-remove-legacy-traitsto one commit on vitamincmain, and takes vitaminc's breaking changes since then.Rules for this branch
rev, neverbranch. A branch is deleted when its stack merges.packages/eqlandpackages/stack-encrypt/fuzz. Two commits mean two copies of vitaminc's traits, and the code stops compiling.Changes
Verification
nextwas created frommainat 2a70a47. Nothing has been built yet, because nothing has changed.Related