[1/1] chore: configure agent skill conventions#415
Closed
freshtonic wants to merge 9 commits into
Closed
Conversation
…n docs Record where issues live and how domain documentation is laid out, so engineering skills read from the right places instead of assuming defaults. - Issue tracker is Linear (Product Engineering, CIP-), not GitHub Issues. GitHub holds code and PRs only. Prefers linear-cli for writes since the Linear MCP tools are unavailable in headless runs. - Domain docs are multi-context: CONTEXT-MAP.md points at per-package CONTEXT.md files under packages/*/. Per-package glossaries and ADRs are created lazily, so their absence is expected. No triage labels recorded; the triage skill is not installed.
📝 WalkthroughWalkthroughThe pull request migrates proxy encryption contracts and ZeroKMS processing from EQL v2 to v3, updates dependencies and EQL configuration, and adds agent-facing workspace, domain, issue-tracking, triage, and planning documentation. ChangesEQL v3 migration
Agent documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/agents/domain.md`:
- Around line 28-39: Update the fenced tree block in the domain documentation by
adding an appropriate language identifier, preferably text, to its opening fence
while leaving the directory tree content unchanged.
- Around line 56-61: Update the “Flag ADR conflicts” example in the relevant
documentation to remove the unrelated “ADR-0007 (event-sourced orders)”
reference. Replace it with a generic ADR placeholder or an actual CipherStash
ADR while preserving the guidance to surface conflicts explicitly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b015cd73-40cb-4798-ab8b-0f80f091a256
📒 Files selected for processing (5)
CLAUDE.mdCONTEXT-MAP.mddocs/agents/domain.mddocs/agents/issue-tracker.mddocs/agents/triage-labels.md
Comment on lines
+28
to
+39
| ``` | ||
| / | ||
| ├── CONTEXT-MAP.md | ||
| ├── docs/adr/ ← system-wide decisions | ||
| └── packages/ | ||
| ├── cipherstash-proxy/ | ||
| │ ├── CONTEXT.md | ||
| │ └── docs/adr/ ← context-specific decisions | ||
| └── eql-mapper/ | ||
| ├── CONTEXT.md | ||
| └── docs/adr/ | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to the fenced tree block.
Markdownlint reports the opening fence at Line 28 without a language. Use text or another appropriate language identifier.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for 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.
In `@docs/agents/domain.md` around lines 28 - 39, Update the fenced tree block in
the domain documentation by adding an appropriate language identifier,
preferably text, to its opening fence while leaving the directory tree content
unchanged.
Source: Linters/SAST tools
Comment on lines
+56
to
+61
| ## Flag ADR conflicts | ||
|
|
||
| If your output contradicts an existing ADR, surface it explicitly rather than silently | ||
| overriding: | ||
|
|
||
| > _Contradicts ADR-0007 (event-sourced orders) — but worth reopening because…_ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a repository-relevant ADR example.
“ADR-0007 (event-sourced orders)” does not match this repository’s documented domains and may cause agents to produce misleading conflict references. Use a generic placeholder or an actual CipherStash ADR.
🤖 Prompt for 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.
In `@docs/agents/domain.md` around lines 56 - 61, Update the “Flag ADR conflicts”
example in the relevant documentation to remove the unrelated “ADR-0007
(event-sourced orders)” reference. Replace it with a generic ADR placeholder or
an actual CipherStash ADR while preserving the guidance to surface conflicts
explicitly.
Resolve the terms that collide hardest across the two contexts, and correct the Index entry in CONTEXT-MAP.md, which had the collision backwards. - EqlTrait is a capability; the storage that satisfies it is a SEM term (searchable encrypted metadata), many-to-one. Neither is an index. - Ban 'session' in Proxy: the code used it for both a connection and a single statement, and PostgreSQL owns the word for a third thing. - Split EncryptConfig into encrypt credentials vs column encrypt config; the single name currently covers two unrelated public structs. - Record that Proxy assigns EqlTraits::all() to every encrypted column, so EQL Mapper's bound checking cannot fire in production.
Knowingly leaves the workspace not compiling. Fixing the fallout is deliberately deferred. - cipherstash-client / cipherstash-config / cts-common: 0.34.1-alpha.4 -> 0.42.0 - CS_EQL_VERSION: eql-2.3.0-pre.3 -> eql-3.0.1 (major, breaking) - Drop the CIP-3159 vendored stack-auth patch: 0.42.0 requires stack-auth ^0.42.0, which carries the CancelGuard fix upstream. vendor/stack-auth is left in place but is now unreferenced.
Switch the encrypt path to cipherstash-client 0.42.0's encrypt_eql_v3 and make v3 the only wire envelope Proxy speaks. Still does not compile; threading the new types through the protocol layer is deferred. - lib.rs re-exports the V3 types under the old names and deliberately does NOT re-export the v2 ones, so any remaining v2 use fails to resolve rather than silently writing v2 payloads. - EQL_SCHEMA_VERSION 2 -> 3; startup gate now queries eql_v3.version() (eql_v2 schema no longer exists in EQL 3). - Decrypt is now local: there is no decrypt_eql_v3 in the client, but a scalar payload's 'c' is an EncryptedRecord and EncryptedRecord is Decryptable, so it goes straight to the cipher. - SteVec (jsonb) decrypt returns SteVecV3DecryptUnsupported: v3 stores the key header once at the document root with raw AEAD bytes per entry, and reassembling that here would hard-code the envelope layout.
Follows the approach protect-ffi uses (encrypted_record_from_value / v3_root_record): skip decrypt_eql entirely and hand an EncryptedRecord straight to the cipher. Replaces the SteVecV3DecryptUnsupported error added in the previous commit, which was wrong -- KeyHeader::record_with_selector is public and does exactly the reassembly needed. Scalar and SteVec records are NOT interchangeable, hence the V3Record enum: RecordWithNonce unconditionally reports a nonce override and an AAD selector, so wrapping a scalar record in one would decrypt against a nonce the value was never encrypted with. Note protect-ffi's SteVec branch does not port directly: it pins cipherstash-client =0.41.1, where sv entries are self-describing mp_base85 records. 0.42.0 moved to an envelope -- key material once in the document's 'h' header, raw AEAD bytes per entry, nonce and AAD derived from the entry selector.
Completes the v3 move. The workspace compiles again; cargo fmt and
clippy are clean.
The load-bearing change is in data_row.rs. EQL v2's eql_v2_encrypted was
a COMPOSITE type, so reads stripped a ('...') wrapper in text and a
12-byte rowtype header in binary. EQL v3's types are DOMAINS over jsonb
(95 CREATE DOMAIN, zero composites), and a domain is wire-identical to
its base type -- so both of those strips were wrong. Reads now take the
jsonb representation: bare JSON in text, a 1-byte version header plus
JSON in binary.
Also:
- Encrypt now carries EqlOutput, not EqlCiphertext, through the literal
and param paths: a query operand is not a ciphertext. Both v3 enums
are #[serde(untagged)], so the serialized wire form is unchanged.
- backend/bind/data_row imported cipherstash_client::eql::EqlCiphertext
directly, bypassing the crate alias and silently keeping the v2 type.
They now use crate::EqlCiphertext.
- EqlCiphertext::identifier is a method in 0.42.0, not a field.
- Map the new EqlError variants. UnsupportedSteVecOreInV3 gets its own
customer-facing error: v3 has no oc representation, so a ste_vec column
left in Standard (CLLW-ORE) mode cannot be written at all.
- cipherstash-config gained IndexType::SteVec { mode }. SteVecMode::Compat
(CLLW-OPE) is the default and the v3-compatible mode; Standard is
documented upstream as the legacy v2 protocol.
- CouldNotDecryptDataForKeyset gained a #[source]; Proxy's variant does
not carry it, so the chain stops at that boundary (noted in place).
Captures the EQL v3 domain model, three impact maps over eql-mapper (type system, SQL surface, declarations/macros), and the open design questions, so the design session starts fresh with full context. Records one corrected finding: literals are inference sinks (value.rs:19 unifies them with a fresh tvar, not Native), so a token type on EqlValue buys no literal checking. Two subagents disagreed on this; resolved by reading the code.
freshtonic
changed the title
This was referenced Jul 21, 2026
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs (1)
93-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
decode_ste_vec_selector.Pure function, no KMS/async dependency — cheap to cover valid 16-byte hex, invalid hex, and wrong-length inputs, which are exactly the boundary cases that would otherwise surface as decrypt failures in production.
🤖 Prompt for 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. In `@packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs` around lines 93 - 105, Add unit tests for the private function decode_ste_vec_selector covering valid 16-byte hexadecimal input, invalid hexadecimal input, and valid hexadecimal input with an incorrect decoded length. Assert successful decoding for the valid case and EncryptError::SteVecSelectorInvalid for both failure cases.
🤖 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 `@docs/plans/2026-07-20-eql-v3-type-checker-handoff.md`:
- Around line 393-397: Remove the machine-specific ~/.claude/projects/...
reference from the “Related memory” section. Replace it with links to
repository-contained documentation or portable descriptions of the listed
artifacts, without exposing developer-local filesystem paths.
- Around line 26-35: Correct the commit count in the introductory sentence above
the commit table to match the six listed commits, preserving all existing commit
rows and descriptions.
- Around line 37-40: Update the build-status section to associate the “workspace
compiles” and clean cargo fmt/cargo clippy results explicitly with commit
387ca790. Replace or scope “Nothing has been run” so it accurately describes the
commands and results for the current handoff state without conflating it with
that commit’s status.
- Around line 237-261: Revise the “entire v2 SQL surface” inventory to
distinguish the six transformation rules from their shared emitters: identify
four rules that emit SQL directly, and describe helpers.rs as the shared cast
helper used by two of them rather than as an additional call site. Preserve the
listed symbols and migration hooks while making the total migration scope
unambiguous.
In `@packages/cipherstash-proxy/src/error.rs`:
- Around line 264-272: Update the #[error] messages for SteVecMissingRootEntry
and SteVecSelectorInvalid in EncryptError to include the standard
customer-facing “For help visit {}#...” documentation-link fragment, using the
same formatting and relevant anchors as sibling SteVec* variants such as
SteVecOreModeUnsupported.
In `@packages/eql-mapper/CONTEXT.md`:
- Around line 3-5: The EQL Mapper context still references retired EQL v2
behavior. In packages/eql-mapper/CONTEXT.md lines 3-5, update the mapper
description to say it rewrites statements into EQL v3 equivalents or explicitly
identify the behavior as legacy; in lines 85-87, update the TransformationRule
documentation to describe EQL v3 operations.
---
Nitpick comments:
In `@packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs`:
- Around line 93-105: Add unit tests for the private function
decode_ste_vec_selector covering valid 16-byte hexadecimal input, invalid
hexadecimal input, and valid hexadecimal input with an incorrect decoded length.
Assert successful decoding for the valid case and
EncryptError::SteVecSelectorInvalid for both failure cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e0d824d-9dbb-4874-a525-692bb6f2f718
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
CONTEXT-MAP.mdCargo.tomldocs/plans/2026-07-20-eql-v3-type-checker-handoff.mdmise.local.example.tomlmise.tomlpackages/cipherstash-proxy/CONTEXT.mdpackages/cipherstash-proxy/src/error.rspackages/cipherstash-proxy/src/lib.rspackages/cipherstash-proxy/src/postgresql/backend.rspackages/cipherstash-proxy/src/postgresql/context/mod.rspackages/cipherstash-proxy/src/postgresql/frontend.rspackages/cipherstash-proxy/src/postgresql/messages/bind.rspackages/cipherstash-proxy/src/postgresql/messages/data_row.rspackages/cipherstash-proxy/src/proxy/encrypt_config/manager.rspackages/cipherstash-proxy/src/proxy/mod.rspackages/cipherstash-proxy/src/proxy/zerokms/zerokms.rspackages/eql-mapper/CONTEXT.mdpackages/eql-mapper/src/type_checked_statement.rs
Comment on lines
+26
to
+35
| Five commits, unpushed beyond #415's first. In order: | ||
|
|
||
| | Commit | What | | ||
| |---|---| | ||
| | `3d33c8ed` | Agent skills config (issue tracker, domain docs) | | ||
| | `74b63721` | Domain glossaries for Proxy and EQL Mapper | | ||
| | `64c658f2` | Deps: cipherstash-client `0.34.1-alpha.4` → `0.42.0`, EQL `2.3.0-pre.3` → `3.0.1` | | ||
| | `b3f293d7` | Encrypt path moved to `encrypt_eql_v3`; v2 retired | | ||
| | `a10b60cd` | v3 decrypt for scalar + SteVec | | ||
| | `387ca790` | Adopted the 0.42.0 representation; workspace compiles | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the commit count.
The heading says “Five commits,” but the table contains six commit rows. Update the count or the table.
🤖 Prompt for 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.
In `@docs/plans/2026-07-20-eql-v3-type-checker-handoff.md` around lines 26 - 35,
Correct the commit count in the introductory sentence above the commit table to
match the six listed commits, preserving all existing commit rows and
descriptions.
Comment on lines
+37
to
+40
| **State:** the workspace compiles, `cargo fmt` and `cargo clippy --all-targets` are clean. | ||
| **Nothing has been run.** Compiling is not working — `eql-mapper` still emits `eql_v2_encrypted` | ||
| casts and `eql_v2.*` calls into every rewritten statement, so nothing works end to end. | ||
| 229 `eql_v2` references remain repo-wide. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
# Locate and inspect the plan document around the referenced lines.
wc -l docs/plans/2026-07-20-eql-v3-type-checker-handoff.md
cat -n docs/plans/2026-07-20-eql-v3-type-checker-handoff.md | sed -n '1,120p'Repository: cipherstash/proxy
Length of output: 6146
🏁 Script executed:
set -euo pipefail
# Search for the cited status phrases and nearby build-state notes in the repo.
rg -n --hidden --no-messages -S \
"workspace compiles|cargo fmt|cargo clippy|Nothing has been run|Compiling is not working|eql_v2_encrypted|eql_v2\\.|229 `?eql_v2`? references" \
docs/ .Repository: cipherstash/proxy
Length of output: 28449
🏁 Script executed:
set -euo pipefail
# Inspect the nearby sections that describe the branch state and any commands/results.
sed -n '24,50p' docs/plans/2026-07-20-eql-v3-type-checker-handoff.md
printf '\n---\n'
sed -n '140,220p' docs/plans/2026-07-20-eql-v3-type-checker-handoff.md
printf '\n---\n'
sed -n '220,290p' docs/plans/2026-07-20-eql-v3-type-checker-handoff.mdRepository: cipherstash/proxy
Length of output: 8654
Clarify the build-state timeline. Tie the workspace compiles and clean cargo fmt / cargo clippy --all-targets status to 387ca790, and scope or remove Nothing has been run so the handoff records the actual commands/results instead of mixing states.
🤖 Prompt for 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.
In `@docs/plans/2026-07-20-eql-v3-type-checker-handoff.md` around lines 37 - 40,
Update the build-status section to associate the “workspace compiles” and clean
cargo fmt/cargo clippy results explicitly with commit 387ca790. Replace or scope
“Nothing has been run” so it accurately describes the commands and results for
the current handoff state without conflating it with that commit’s status.
Comment on lines
+237
to
+261
| ### The entire v2 SQL surface is three call sites | ||
|
|
||
| Six rules, composed at `type_checked_statement.rs:153-160`. Only three emit SQL. | ||
|
|
||
| **`helpers.rs:6-27`** — `cast_as_encrypted`. The single source of every | ||
| `::JSONB::eql_v2_encrypted` cast. Takes **only** an `ast::Value` — no `EqlTerm`, no | ||
| `TableColumn`, no traits. The cast target is a hardcoded `Ident::new("eql_v2_encrypted")` | ||
| at line 17. **This is the one place the v3 domain name would be chosen.** | ||
|
|
||
| **`cast_params_as_encrypted.rs:51`** — emits `$1::JSONB::eql_v2_encrypted`. Its gate at line 67 | ||
| matches `Some(Type::Value(Value::Eql(_)))` — **the `EqlTerm` is available and discarded by the | ||
| `_`, one line before use.** That is the hook for selecting a `query_<name>` twin. | ||
|
|
||
| **`cast_literals_as_encrypted.rs:33`** — emits `'<ciphertext>'::JSONB::eql_v2_encrypted`. Gated | ||
| on map membership, not type. Holds **no `node_types` field at all** (line 13), so it cannot see | ||
| type info even in principle. Needs plumbing, not just a pattern change. | ||
|
|
||
| **`rewrite_standard_sql_fns_on_eql_types.rs:59-61`** — blindly prepends `eql_v2` to any | ||
| `pg_catalog.*` name. Emits `eql_v2.{count,min,max,jsonb_path_query,jsonb_path_query_first, | ||
| jsonb_path_exists,jsonb_array_length,jsonb_array_elements,jsonb_array_elements_text}`. | ||
| Note it emits `eql_v2.count`, which **is not in the declaration table** — a name the type | ||
| registry cannot round-trip. | ||
|
|
||
| **`rewrite_containment_ops.rs:54-57,86-87`** — `@>` → `eql_v2.jsonb_contains`, | ||
| `<@` → `eql_v2.jsonb_contained_by`. Motivated by GIN index usage, not just type dispatch. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '220,280p' docs/plans/2026-07-20-eql-v3-type-checker-handoff.mdRepository: cipherstash/proxy
Length of output: 3328
🏁 Script executed:
rg -n "cast_as_encrypted|cast_params_as_encrypted|cast_literals_as_encrypted|rewrite_standard_sql_fns_on_eql_types|rewrite_containment_ops|preserve_effective_aliases|fail_on_placeholder_change" -S .Repository: cipherstash/proxy
Length of output: 3461
🏁 Script executed:
sed -n '1,220p' packages/eql-mapper/src/transformation_rules/mod.rs && printf '\n---\n' && sed -n '1,120p' packages/eql-mapper/src/transformation_rules/helpers.rs && printf '\n---\n' && sed -n '1,120p' packages/eql-mapper/src/transformation_rules/cast_params_as_encrypted.rs && printf '\n---\n' && sed -n '1,120p' packages/eql-mapper/src/transformation_rules/cast_literals_as_encrypted.rs && printf '\n---\n' && sed -n '1,160p' packages/eql-mapper/src/transformation_rules/rewrite_standard_sql_fns_on_eql_types.rs && printf '\n---\n' && sed -n '1,180p' packages/eql-mapper/src/transformation_rules/rewrite_containment_ops.rsRepository: cipherstash/proxy
Length of output: 18763
Clarify the SQL-surface count. This section mixes rules and shared emitters: there are six transformation rules, four of them emit SQL directly, and helpers.rs is a shared cast helper used by two of those rules. Reword the inventory so the migration scope is unambiguous.
🤖 Prompt for 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.
In `@docs/plans/2026-07-20-eql-v3-type-checker-handoff.md` around lines 237 - 261,
Revise the “entire v2 SQL surface” inventory to distinguish the six
transformation rules from their shared emitters: identify four rules that emit
SQL directly, and describe helpers.rs as the shared cast helper used by two of
them rather than as an additional call site. Preserve the listed symbols and
migration hooks while making the total migration scope unambiguous.
Comment on lines
+393
to
+397
| ## 9. Related memory | ||
|
|
||
| `~/.claude/projects/-Users-jamessadler-cipherstash-proxy/memory/`: | ||
| `eql-3-upgrade-in-flight.md`, `cip-3233-client-038-ore-cllw-break.md`, | ||
| `proxy-222-stackauth-15min-auth-bug.md`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the machine-specific memory path.
The ~/.claude/projects/... reference is not portable across contributors and exposes a developer-local filesystem layout. Link to repository-contained documentation or describe these artifacts without a local path.
🤖 Prompt for 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.
In `@docs/plans/2026-07-20-eql-v3-type-checker-handoff.md` around lines 393 - 397,
Remove the machine-specific ~/.claude/projects/... reference from the “Related
memory” section. Replace it with links to repository-contained documentation or
portable descriptions of the listed artifacts, without exposing developer-local
filesystem paths.
Comment on lines
+264
to
+272
| /// `sv[0]` is the decryption root of a SteVec document, so an empty `sv` | ||
| /// array leaves nothing to decrypt. | ||
| #[error("Encrypted jsonb value has no root entry and cannot be decrypted")] | ||
| SteVecMissingRootEntry, | ||
|
|
||
| /// A SteVec entry's selector is the source of both AEAD bindings (nonce and | ||
| /// AAD), so it must be exactly 16 hex-encoded bytes. | ||
| #[error("Encrypted jsonb entry has an invalid selector '{selector}'")] | ||
| SteVecSelectorInvalid { selector: String }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Missing documentation links on new SteVec* error variants.
SteVecMissingRootEntry and SteVecSelectorInvalid lack the "For help visit {}#..." fragment that every other EncryptError variant (including the sibling SteVecOreModeUnsupported added in this same diff) provides.
🛠️ Proposed fix
- #[error("Encrypted jsonb value has no root entry and cannot be decrypted")]
+ #[error("Encrypted jsonb value has no root entry and cannot be decrypted. For help visit {}`#encrypt-ste-vec-missing-root-entry`", ERROR_DOC_BASE_URL)]
SteVecMissingRootEntry,
/// A SteVec entry's selector is the source of both AEAD bindings (nonce and
/// AAD), so it must be exactly 16 hex-encoded bytes.
- #[error("Encrypted jsonb entry has an invalid selector '{selector}'")]
+ #[error("Encrypted jsonb entry has an invalid selector '{selector}'. For help visit {}`#encrypt-ste-vec-selector-invalid`", ERROR_DOC_BASE_URL)]
SteVecSelectorInvalid { selector: String },As per coding guidelines, packages/**/*.rs errors should "provide customer-facing messages with documentation links."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// `sv[0]` is the decryption root of a SteVec document, so an empty `sv` | |
| /// array leaves nothing to decrypt. | |
| #[error("Encrypted jsonb value has no root entry and cannot be decrypted")] | |
| SteVecMissingRootEntry, | |
| /// A SteVec entry's selector is the source of both AEAD bindings (nonce and | |
| /// AAD), so it must be exactly 16 hex-encoded bytes. | |
| #[error("Encrypted jsonb entry has an invalid selector '{selector}'")] | |
| SteVecSelectorInvalid { selector: String }, | |
| /// `sv[0]` is the decryption root of a SteVec document, so an empty `sv` | |
| /// array leaves nothing to decrypt. | |
| #[error("Encrypted jsonb value has no root entry and cannot be decrypted. For help visit {}`#encrypt-ste-vec-missing-root-entry`", ERROR_DOC_BASE_URL)] | |
| SteVecMissingRootEntry, | |
| /// A SteVec entry's selector is the source of both AEAD bindings (nonce and | |
| /// AAD), so it must be exactly 16 hex-encoded bytes. | |
| #[error("Encrypted jsonb entry has an invalid selector '{selector}'. For help visit {}`#encrypt-ste-vec-selector-invalid`", ERROR_DOC_BASE_URL)] | |
| SteVecSelectorInvalid { selector: String }, |
🤖 Prompt for 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.
In `@packages/cipherstash-proxy/src/error.rs` around lines 264 - 272, Update the
#[error] messages for SteVecMissingRootEntry and SteVecSelectorInvalid in
EncryptError to include the standard customer-facing “For help visit {}#...”
documentation-link fragment, using the same formatting and relevant anchors as
sibling SteVec* variants such as SteVecOreModeUnsupported.
Source: Coding guidelines
Comment on lines
+3
to
+5
| Parses SQL, infers a type for every node, and rewrites statements that touch encrypted | ||
| columns into their EQL v2 equivalents. It knows nothing about the PostgreSQL wire | ||
| protocol, ZeroKMS, or ciphertext — it reasons about types and rewrites syntax. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the EQL Mapper context with the v3 migration. The glossary still describes the mapper as producing EQL v2 operations, although the handoff states that EQL v2 is retired.
packages/eql-mapper/CONTEXT.md#L3-L5: describe rewriting into EQL v3 equivalents, or explicitly mark this as legacy behavior.packages/eql-mapper/CONTEXT.md#L85-L87: updateTransformationRuleto document EQL v3 operations.
📍 Affects 1 file
packages/eql-mapper/CONTEXT.md#L3-L5(this comment)packages/eql-mapper/CONTEXT.md#L85-L87
🤖 Prompt for 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.
In `@packages/eql-mapper/CONTEXT.md` around lines 3 - 5, The EQL Mapper context
still references retired EQL v2 behavior. In packages/eql-mapper/CONTEXT.md
lines 3-5, update the mapper description to say it rewrites statements into EQL
v3 equivalents or explicitly identify the behavior as legacy; in lines 85-87,
update the TransformationRule documentation to describe EQL v3 operations.
This was referenced Jul 21, 2026
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.
📚 Queued PR · 1 of 1
Part of a queue. The PRs merge in FIFO order — the numbered order below, #1 first. Merging one supersedes the PRs after it until the author runs
git queue sync(rebases the rest onto the merged base) andgit queue submit(retargets their PRs).chore/agent-skills-setup→main👈 this PR✅ approved · ♻️ changes requested · ⏳ review pending | 🟣 merged · 🟢 open · ⚫ closed — status as of the last
git queue submit.🥞 Managed by git-queue — do not edit this list by hand.
Configures this repo for the mattpocock engineering skills — the per-repo conventions those skills read before doing anything.
No product code is touched. Docs and
CLAUDE.mdonly.What's here
Issue tracker (
docs/agents/issue-tracker.md) — records that issues live in Linear (team Product Engineering,CIP-prefix), not GitHub Issues. Covers thelinear-cli/ MCP access paths, the workflow statuses, and how each skill should move an issue through them. Also pins the convention that markdown goes via--description-file, since inline args render literal\nin the Linear UI.Triage labels (
docs/agents/triage-labels.md) — maps the five canonical triage roles to our label strings. Kept as the defaults. Onlywontfixexists on the team today; the other four are created on demand, and the file notes they should stay unparented rather than joining the existingProduct/R&D/Issue typegroups.Domain docs (
CONTEXT-MAP.md,docs/agents/domain.md) — multi-context layout, since this is a Cargo workspace with genuinely distinct vocabularies per package. The map names the four contexts and, more usefully, records the cross-context term collisions: "type" means a wire-protocol type in Proxy but an inferred SQL expression type in EQL Mapper; "index" means a Postgres index in Proxy but a searchable-encryption index in EQL Mapper.Per-package
CONTEXT.mdfiles are deliberately not created upfront — they're written lazily as terms actually get resolved, so a missing one is expected rather than a gap.Note
No
CIP-ticket — this is repo configuration rather than product work.Summary by CodeRabbit