[coderdan]

Repeated Rust .extend(a).extend(b) calls previously discarded a, producing a different context from Go. Extensions now append in order, preserve grouped parts, and treat an empty extension as a no-op.

This is a breaking context change. Queries using repeated extensions silently miss rows written under replacement semantics, without returning an error. Keep the final extension alone for both reads and queries of old data, or re-encrypt and re-index it under the appended context. A regression test pins compatible opening and querying, including the silent miss with the new repeated-extension query.

Adds required Rust/Go context conformance: 18 composition cases, 11 read/preflight cases and four query cases. Tests cover typed Rust chains, generated Go literal/field-base APIs, late authentication failure without partial results, and the actual plan bytes sent by Go to WASM. Native Rust checks canonical descriptors/AAD/PRF bytes and retrieval counts; Go checks the outgoing plan, authenticated cross-opening and the uninitialised guest's preflight boundary. Existing context-field fixtures and native plan-grammar tests are enrolled too.

Validation: full mise run test:conformance with fresh artifact exchange; serde_json/preserve_order; the focused several_parts_nest_to_the_left regression; Rust clippy and formatting; Go lint and generated bindings; CI wiring and main-guard tests.

Closes #1475
Fixes #1120

[changeset-bot]

⚠️ No Changeset found

Latest commit: d2a0ef3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5e80de2f-1f00-45c5-960a-20d244ff7244

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

[github-actions]

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

caught	missed	unviable	timeout
1	0	0	0

Every mutant in the changed lines was caught by a test.

[coderdan]

Review: align and test context composition

Reviewed 355f62138 against next, on two axes: the repo's coding standards, and #1475, #1120 and the conformance plan. The core fix is correct: .extend() now appends through DeclaredContext::append (src/target/context.rs:188), and tests/plan.rs:560-586 would fail if that body did nothing. Locally context_conformance passed 4/4, plan 31/31, and stack-conformance-ci 7/7.

Standards

Against documented standards:

1. The behaviour change is recorded as a fix, not a breaking change. stack-encrypt is published on crates.io, and this changes the context that existing .extend(a).extend(b) call sites produce. The commit is fix(stack-encrypt): with no !, and the CHANGELOG entry is under ### Fixed (CHANGELOG.md:10-16). It should be fix(stack-encrypt)!: with the bullet under ### Breaking.
2. An issue number in a code comment. tests/context_conformance.rs:307: // A no-op extension must not clear earlier calls (#1120). Say what the behaviour is instead.
3. Issue numbers in the contract doc. conformance/stack-encrypt/context-contract.md:37, 63, 108 cite stack-encrypt: a second .extend() replaces the first, so Rust builds a different context than Go and a query silently misses #1120 and stack-encrypt refactor: separate ciphertext binding from search scope #1487. It's reference documentation beside a README, so the same rule applies.

Judgement calls:

• Issue numbers in the plan doc. docs/plans/2026-10-31-stack-encrypt-conformance.md:214-224 adds four. The file already cites PRs throughout, so this is borderline. There is also a double blank line before the new heading.
• Duplicated code. tests/context_conformance.rs:77 and :90 build the same nested frame with the same .fold(frame(...), |head, p| frame(vec![head, part(p)])). A nest(head, &case.parts) helper would serve both.
• Four edits to enroll two Go tests. TestContextConformance and TestContextBoundaries are added in scripts/stack-conformance-selection.mjs, twice in scripts/__tests__/stack-conformance-ci.test.mjs, and in fixture_conformance_test.go. The CI test could import the list from the selection module.
• A test name that says less than it checks. enrolled_context_tests_are_registered (tests/record_lowering.rs:844) checks every fixture test in record_lowering, not only context ones.
• A comment that reads as the opposite of the code. context_field_test.go:236 says "Required conformance must fail if its guest is absent" above a helper whose first call is skipMissingConformanceGuest. "Skips unless conformance is required" would match.

Spec

Verified: an empty extension is a no-op in both languages. Rust runs .extend(()) after each typed write, query and open; Go compares .Extend() with repeated calls. All 18 composition cases run in both Rust and Go: Go seals fresh rows, compares terms, opens Rust's ciphertexts and builds queries. Rust really counts key retrievals (counting_cipher). Required-mode Go tests fail rather than skip, and CI selects the new tests.

(a) Missing or partial:

• "Compare canonical descriptors/AAD" happens in Rust only. Go checks that the frozen descriptor, AAD and PRF are non-empty (context_conformance_test.go:129). Its frame comparison encodes a hand-built plan through FieldContext, not what the cipher actually sent. The file says why (Go has no public encoder), but the comparison is one-sided.
• "Cover … queries, and empty/late-failure batches." context-boundaries.json has no query cases. Every late failure is a refusal before any key is fetched, and both tampering cases are single-row. Nothing covers a failure after keys are retrieved, partway through a batch.
• "Exercise typed/generated APIs." Rust's typed chains skip the two empty-part cases (context_conformance.rs:289). Go's generated Note methods run only for field-derived cases (:169), so no literal-base generated type is exercised. The boundary cases go through dynamic lowering only.
• "Rejection before key retrieval." Go only checks that the corpus retrievals values agree with the outcome. Its real evidence is a preflight with an uninitialised checker, which is a reasonable stand-in.
• stack-encrypt: a second .extend() replaces the first, so Rust builds a different context than Go and a query silently misses #1120 item 2 asked for a case in several_parts_nest_to_the_left. That test is unchanged. The intent is covered by the literal-ordered typed chain and a_second_extend_appends_to_the_first.

(b) Scope creep: none of substance. Enrolling plan_grammar and record_lowering is related work.

(c) Implemented but looks wrong:

• stack-encrypt: a second .extend() replaces the first, so Rust builds a different context than Go and a query silently misses #1120 item 3 (CHANGELOG): filed under Fixed rather than Breaking, as in Standards item 1.
• The CHANGELOG leaves out the bigger hazard. It says old records "must still be opened with the final extension alone". It doesn't say that a query built with repeated .extend() now matches nothing on rows written before the fix, and returns no error. That is the silent miss stack-encrypt: a second .extend() replaces the first, so Rust builds a different context than Go and a query silently misses #1120 reported, now hitting old data.
• The old-records claim has no test. It holds by construction, but no test opens a record written under the replacement behaviour.
• Tampering behaves differently in each language. The corpus has one outcome, "authentication", but Rust fails AEAD after one key retrieval while Go returns ErrForbidden. context-contract.md documents this. The corpus's "one retrieval" is checked against real calls only in Rust.

Summary

• Standards: 3 against documented standards and 5 judgement calls. The worst is the behaviour change being released as a fix, with no ! and no Breaking entry.
• Spec: 5 partial requirements and 4 that look wrong. The worst is that the CHANGELOG doesn't warn that repeated-.extend() queries now silently miss rows written before the fix.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-01T09:54:19.080962Z	355f621	Draft marked ready
🔒 Security Review	✅ Completed 2026-11-01T09:55:50.859355Z	355f621	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[cipherstash-agent]

Automated review attestation (/code-review medium + cross-check against existing feedback)

Reviewed head 355f621 (diff read in full for source, CHANGELOG, CI wiring and Go test changes; conformance corpora and Rust test bodies skimmed, not executed).

Result: no critical or blocking issues. No new findings.

• DeclaredContext::append (src/target/context.rs) preserves per-part grouping, and .extend(()) appends an empty vec, so it is a no-op. The macro-generated extend in plan/chain.rs is the only place that assigned common.extend, so no other call site still replaces.
• Test and CI wiring look consistent: the new Rust test binaries are in stack-conformance.sh, and the Go tests are in both the selection list and the guest-required list.
• Candidate concerns were cross-checked against the existing review (comment) and dropped as already covered: breaking-change classification and CHANGELOG placement, missing warning that repeated-.extend() queries silently miss pre-fix rows, no test opening a record written under replacement semantics, and issue numbers in comments and docs.
• Minor, optional: docs/plans/2026-10-04-plan-builder.md:1040 still says "Cipher.Extend replaces the last". It is a historical plan, so it may be fine to leave.

The Changeset bot reports no changeset. AGENTS.md asks for one when a published package's behaviour changes, but stack-encrypt is a Rust crate versioned by its CHANGELOG, so I did not raise it as a finding.

---

Generated by Claude Code

[coderdan]

Addressed the review findings in d2a0ef3.

• Breaking change and migration: the follow-up commit and PR title use fix(stack-encrypt)!:, and the changelog entry is under Breaking. The changelog, contract and PR description warn that repeated-extension queries silently miss old rows. A regression reproduces the old surviving-part context, opens it with the final extension alone, verifies its compatible query, and proves the new repeated-extension query succeeds with a different term.
• Coverage: added missing/invalid/matching/other-tenant query cases, a late context-tampering batch, and a late ciphertext-tampering batch that reaches AEAD in both languages and returns no partial rows. Boundary cases now also exercise typed Rust opening and generated Go opening. Generated LiteralNote covers every literal-base composition case. The requested several_parts_nest_to_the_left test now compares a repeated-extension typed chain with lowering.
• Actual Go input: a test-only wrapper observes the plan buffer passed to se_encrypt_record. The test compares its context against a frozen native plan-context frame, replacing the independently reconstructed plan and non-empty AAD checks. Existing frozen ciphertexts and terms are unchanged. Go’s evidence for AAD remains actual-wire agreement plus authenticated cross-opening; native Rust directly checks descriptor/AAD/PRF bytes.
• Explicit API limits: the native empty-part cases assert constructor rejection. An absent member remains a raw-tree case because a generated Go struct cannot represent it. The corpus field is now native_retrievals; Go continues to prove preflight through the uninitialised guest. The provider-specific tampered-context errors remain documented separately from the new shared AEAD-failure case.
• Cleanup: removed the cited issue references from code/reference docs and the new plan section, extracted the nesting helper, shared the JS core-test list with CI tests, renamed the fixture inventory test, clarified the optional/required guest helper, and corrected the historical plan wording about ExtendContext.

Validation passed: full conformance with fresh artifact exchange, ordered-JSON build, focused lowering regression, Rust clippy/formatting, Go lint/generation, and CI wiring/main-guard tests.