fix(eql): the four blockers from #885, and the upstream 3.0.5 sync - #915
Open
tobyhede wants to merge 29 commits into
Open
fix(eql): the four blockers from #885, and the upstream 3.0.5 sync#915tobyhede wants to merge 29 commits into
tobyhede wants to merge 29 commits into
Conversation
Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.3.0 to 4.3.1. - [Changelog](https://github.com/nodeca/js-yaml/blob/4.3.1/CHANGELOG.md) - [Commits](nodeca/js-yaml@4.3.0...4.3.1) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 4.3.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
…aml-4.3.1 build(deps-dev): bump js-yaml from 4.3.0 to 4.3.1
…-alias Restore deprecated ste_vec_contains compatibility aliases
Signed-off-by: Toby Hede <toby@cipherstash.com>
Signed-off-by: Toby Hede <toby@cipherstash.com>
The jsonwebtoken 10.4.0 bump re-resolved winapi-util's windows-sys edge from 0.61.2 down to 0.48.0. winapi-util declares a permissive range (>=0.48.0, <=0.61.*), so both satisfy it and cargo is free to pick either; pin the lockfile back to 0.61.2. Inert in practice - the edge is cfg(windows)-gated and CI is Ubuntu only - but there is no reason to build against three-generation-older bindings. Signed-off-by: Toby Hede <toby@cipherstash.com>
fix(deps): patch postcss to 8.5.26
fix(deps): bump jsonwebtoken to 10.4.0
Version Packages
Two defects that only fire after a merge, found in PR review. THE VERSION HOOK COULD NOT RUN. `release.yml` passes `version: pnpm run version`, which chains `scripts/sync-lockstep-versions.mjs` -> `mise run release:prepare_bindings_assets` -> `cargo run -p eql-codegen`. The `release` job installed pnpm, Node and node-gyp and nothing else; mise is not on GitHub's ubuntu images. `.changeset/` already holds live changesets, so changesets/action takes its version branch on the next push to main regardless of this PR — `pnpm run version` would have died with ENOENT and no Version Packages PR would have been opened at all. `release-version-hook.test.mjs` asserted the wiring existed; nothing asserted the job could execute it. `workflow-mise-setup.test.mjs` now resolves command chains — action input -> npm script -> spawned binary — and requires a mise setup step at a lower index in any job that reaches one. `add_shims_to_path: false` is load-bearing and would not survive a copy-paste from test-eql.yml: mise's shim dir is prepended to PATH for every later step, so mise's pinned Node 22 would shadow setup-node's and `changeset publish` would shell out to its bundled npm 10.x, below the 11.5.1 that OIDC trusted publishing requires. `changeset publish` HAS NO IDEA WHAT IT IS DOING. It publishes every public workspace package whose version is absent from npm — changeset or no changeset — with `Promise.all` and no dependency ordering, and `publishAPackage` returns a failure rather than throwing it. So a package that cannot publish does not stop the siblings that depend on it: they ship pointing at a version nobody can install. `publishBlockers` in release-gate.mjs answers that question before anything publishes and exits non-zero, which fails `gate` and skips `release` entirely. `FROZEN_PUBLISHERS` has to be written down rather than derived: npm exposes no way to ask "would a publish from here be accepted?", and provenance on the last release is evidence about the past, not permission for the next. Its FFI half turns lint-no-ffi-changeset.mjs's unstated assumption — that all seven are already on npm at the workspace version — into a checked one, which is precisely what the hand-applied EQL 3.0.5 bump showed it needed. The third verdict in the range check is NOT SAFE, MERELY NOT WORSE: a dependency being published in the same release is allowed, because that is every ordinary release here, and changesets' lack of ordering leaves a window nothing at this layer can close. What it can do is separate that transient window from a permanent hole. Also: `changeset:version` was a bare `changeset version` that skipped the lockstep sync while AGENTS.md documented it as the release flow, and `changeset:publish` skipped the build the same way. Both now route through the wrappers, and the test states the rule over the scripts rather than over the two names. packages/protect-ffi/Cargo.lock was stale (`cargo metadata --locked` exit 101): it records eql-bindings, whose version sync-lockstep-versions.mjs moves, and nothing refreshed it — `packages/eql`'s lock is refreshed as a side effect of the SQL build, that one by nothing at all. The script now discovers every lock recording the crate from a path and refreshes each.
… that fires
THE CACHE KEYS COULD NOT SEE THE PATH DEP. Phase 3 pointed protect-ffi at
`eql-bindings = { path = "../../../eql/crates/eql-bindings" }`, making it a
compile input to index.node and to the wasm build — but both keys in
build-ffi-binding hashed only `packages/protect-ffi/**`. A src-only edit
there changes no hashed file (path deps carry no lock checksum), so the
cache hits, the cargo step is skipped, and every credentialed job runs a
stale binding. This contradicts the action's own stated invariant, and this
PR demonstrated it: the 3.0.4 -> 4.0.0 -> 3.0.5 bumps moved neither key.
Scoped to `packages/eql/crates/**` rather than the bindings crate alone.
That crate has no path deps of its own TODAY, so the narrow glob would be
exact — and would silently stop being exact the moment it gains one, which
is the same hole one level down. `packages/eql/Cargo.toml` is in because
three of its four siblings inherit `[lints] workspace = true`, so the
workspace root is one key away from being a compile input with no other
trace. The discovery test follows path deps transitively and asserts every
git-tracked file under each is covered, so a future dep is caught the day
it lands rather than the day it breaks.
THE PG 14-16 MATRIX RAN NEVER. test-eql.yml gated the full matrix behind
`merge_group` alone and dropped `push:` on the justification that "under a
required merge queue, push-to-main validation is redundant". Every clause
of that is false here, checked against the live API: `mergeQueue(branch:
"main")` is null, `branches/main/protection` 404s, and the active ruleset
carries no `required_status_checks` rule — so `ci-required` is required by
nothing and `merge_group` is an event this repo never emits. Both failures
are invisible, because an event that is never delivered produces no runs to
notice the absence of.
`setup` is now an EXCLUSION — the full matrix is the default and the PR
fast lane is the special case — so a trigger added later gets more coverage
rather than silently none. Every gated job's `if:` was inverted to match.
`push` carries a paths filter (nothing waits on a push run, so a skip costs
nothing) and `schedule` backstops what the filter cannot see; the expensive
axis is the fan-out, not the trigger.
The fail-fast audit found SEVEN multi-command `run:` blocks across three
workflows lacking `set -e` under `shell: bash {0}`, not the one reported.
The worst is macro-expand-eql's `mise run test:matrix:expand` followed by
`git diff --exit-code` on the files it regenerates: when the regeneration
breaks, the snapshots are untouched, the diff is clean, and the job goes
green having verified that a build which never ran produced no drift.
integration-drizzle and integration-protect-ffi gained the EQL crate paths
because adding them to the WASM cache key makes them WASM build inputs, and
wasm-build-inputs-paths-filter.test.mjs enforces that relationship — both
run build-ffi-binding with `wasm: 'true'` and load protect_ffi_inline.js.
The guard exists so that a mise task invoking cargo cannot go unreached by
CI — "a check nothing invokes reads exactly like a check that passes". Its
scan missed 41 of 73 tasks, so the property held over less than half the
surface it named.
Three blind spots, established empirically against mise in a scratch
project rather than inferred:
* BARE TABLE HEADERS. In a config pulled in by `[task_config].includes`,
every top-level table is a task — `["postgres:up"]`, not
`[tasks."postgres:up"]`, which is a hard parse error there. The regex
required the latter, so tasks/postgres.toml and tasks/fixtures.toml
contributed 0 of their 6.
* FILE TASKS. `includes = ["tasks", ...]` makes every executable under
tasks/** a task. None were parsed. The inclusion rule is the executable
bit and nothing else — it correctly excludes the `source`d helpers, the
`bash tasks/...`-invoked scripts, *.sql and *.txt, while including
githooks/pre-commit (no extension) and xml-to-json.py.
* NAME COLLISIONS. Where a file task's stem is already claimed by a TOML
task, mise keeps the extension: tasks/test/known-failures.sh becomes
`test:known-failures.sh`. Collapsing those would have silently replaced
the task CI actually runs.
MUTATION-PROVEN: deleting bench-eql.yml passed all 14 tests. `test:bench`
is a file task, so removing its only caller was invisible — the exact
"suite goes quiet" failure this file exists to prevent. That case is now a
permanent test: `reachableFrom` takes the workflow list as a parameter, so
the mutation is injected rather than performed, and a SOLE_CALLER_WORKFLOWS
equality recomputes reachability with each workflow removed in turn.
The floors could not have caught any of it — mise.toml alone carries 32
tasks against a `>= 30` floor. Replaced with per-source assertions: every
config in `includes` must contribute at least one task, and so must the
file walk.
Two defects found while fixing it. The naive `mise run` body scan created
FALSE reachability, staling `test:sqlx`'s exemption off a comment in
fixtures.toml; and scriptBodies inlined scripts named in `#MISE sources`
comments, falsely clearing `release:prepare-bindings-assets`. Of 56 `mise
run` occurrences, only 20 are commands.
Four new orphans, all exempted with reasons. `test:lint` is not a reflexive
exemption: it is `cargo fmt --check` scoped to tests/sqlx, and `cargo fmt
--check -v` at the workspace root lists those files, so `test:crates` is a
strict superset. `docs:generate:json` gets a new self-expiring category —
blocked on an unported workflow — which the staleness check will demand be
removed the day _build-docs.yml is ported.
…e subtree THE LINTER NEVER OPENED pnpm-workspace.yaml. It walked every Cargo.toml and package.json, which is where a pin is DECLARED — but pnpm resolves `overrides` and `catalogs` from the workspace file, and that file's own comment says so. An override there moves what actually installs while every `workspace:^` in the tree still reads correct, which is the quietest possible way to reintroduce the skew between the Rust that emits an EQL payload and the SQL that stores one. It now reads `overrides`, `catalog` and `catalogs.<name>`, recurses into nested npm overrides, and matches selector keys (`@cipherstash/eql@<3.0.5`, `pkg>@cipherstash/eql`, yarn's `**/` globs) and aliased values (`npm:@cipherstash/eql@3.0.4`). Two judgements worth recording. A `catalog:` specifier in a manifest stays an OFFENDER — unlike Cargo's `workspace = true`, a catalog holds version ranges, so there is no in-tree answer to defer to and both ends are flagged. And YAML scalars are coerced rather than dropped, because `'@cipherstash/eql': 3.0` parses as the NUMBER 3 under YAML 1.1 — dropping non-strings would drop exactly that pin. `EXPECTED_SOURCES` is the floor the workspace file needs: it declares nothing today, so the only checkable antecedent is that it was opened and parsed. Exit 2 for a source that is missing, unparseable or emptied out, checked before offenders, on the existing contract. AGENTS.md rule 7 was not followed by the absorption itself: this adds the largest package in the repo and neither meta file mentioned it. Adds the Repository Layout entry and a "Working on EQL" section covering the seams where failures are silent — the two-level package path and why root `pnpm test` needs `./packages/**`, `working_directory: packages/eql` for mise (and EQL_ROOT, since `git rev-parse --show-toplevel` now answers the monorepo root), the dead `.github/` deposit, the lockstep version identity and the `mise run build --version` cache-key trap, the path dep and its linter, the CI-workspace-keyed SteVec constants, and publishing not having moved. Corrects two now-false claims in the protect-ffi section, including "the only Rust in this repo". The plan's Phase-5 item for a `lint-no-eql-changeset.mjs` is marked superseded: `FROZEN_PUBLISHERS` in release-gate.mjs is strictly stronger, catching the hand-applied bump that no changeset-side guard could see. It also records that `.changeset/rename-ste-vec-contains.md.deferred` must NOT be restored — it was deleted deliberately in 9b1c44d so the cutover cannot apply the same bump twice, and re-parking that `major` changeset would bump an already-released 3.0.5 to 4.0.0.
…ed space THE BLAST RADIUS WAS OVERSTATED IN ONE DIRECTION AND MISSED ENTIRELY IN ANOTHER. The changeset claimed the rename affects "Supabase / PostgREST, since PostgREST calls functions rather than operators". It does not: `eql_v3.jsonb_contains(jsonb, jsonb)` and `jsonb_contained_by(jsonb, jsonb)` are byte-identical to 3.0.4, and those are the function-form entry points a PostgREST deployment actually calls. The real exposure is hand-written SQL. The schema-wide grant scripts in docs/reference/permissions.md pick the new name up on their next run, and a stale per-function grant fails loudly rather than silently granting nothing. Underneath it sits a larger fact that had never been written down: the installer opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`, so EVERY EQL install drops grants, functional indexes and dependent views — not just this release. That is U-002 in the new upgrade note, and it is the item with real operational consequences. A STALE VENDORED MIGRATION DIRECTORY IS NEVER DETECTED. `migrationHash` is computed over a package's own bytes, so a `migrations/cipherstash/` generated against 1.0.0 is stale but internally intact and passes every integrity check. `materialiseExtensionMigrationPackageIfMissing` skips by existence while `refs/head.json` is re-emitted unconditionally, and that asymmetry is the whole failure mode: the head ref demands an invariant the frozen genesis edge does not provide. On an existing database the upgrade still applies; on a fresh one `db init` refuses with `Operation cipherstash.upgrade-eql-v3-bundle-3.0.5 has class "data" which is not allowed by policy.` — naming neither the directory nor the remedy. There is no in-package lever for that message: the text comes from @prisma-next/target-postgres and the operation id it interpolates is frozen inside a published content-addressed artefact. So the README carries the error string verbatim with the remedy, and a test pins the two together so they cannot drift. The changeset also asserted something false: `runContractSpaceSeedPhase` is called from migration-plan.ts and nowhere else, so `migrate` alone never seeds. Without `migration plan` first, the 3.0.5 directory never reaches disk and `migrate` is a silent no-op leaving the database on the older bundle. Pre-existing since 3.0.2, but this changeset stated it as fact. The fixture is not a committed blob — the 1.0.0 baseline is reconstructed from artefacts already in the tree and PROVED faithful by hashing to sha256:fc495f7f..., the value 1.0.0 published. Content addressing makes that an exact-bytes assertion. The `problems: []` claim was verified load-bearing by tampering a byte and confirming hashMismatch fires.
Six open Dependabot alerts, all reachable because the overrides written to close earlier ones had gone stale in two distinct ways. OVERTAKEN SELECTORS go quiet. `postcss@<8.5.10` stopped matching once the tree resolved 8.5.14, so the entry sat here looking like a fix while a vulnerable version installed underneath it (#167 GHSA-r28c-9q8g-f849 HIGH, path traversal reading arbitrary .map files via attacker-controlled sourceMappingURL; #176 its incomplete-fix follow-up). `js-yaml@<3.15.0` was overtaken by its own fix — the tree resolved exactly 3.15.0, so nothing matched and nothing lifted it to 3.15.1 (#189). EXACT PINS INSIDE A STILL-MATCHING RANGE are worse: they cannot self-heal and Dependabot cannot fix them. `js-yaml@>=4.0.0 <5` kept matching, so a pin written to FIX an advisory was holding the tree at 4.2.0 and blocking 4.3.1 (#155, #188 — quadratic-CPU DoS via merge-key chains, then `!!omap` duplicate-key detection). `fast-uri@<3.1.3` did the same at 3.1.3 while #158 needs 3.1.4 and #175 needs 3.1.5. Both shapes are now written into the comments, because the lesson is reusable: keep the ceiling AHEAD of the patched floor rather than at it. THE js-yaml CAUTION IS DISCHARGED, checked rather than assumed. The comment warned that a past bump broke changesets. @changesets/parse@0.4.3 declares `js-yaml: ^4.1.1`, which 4.3.1 satisfies; 4.2.0 and 4.3.1 have identical `dependencies` and `exports`, no `engines`, no `main`; 4.3.0 and 4.3.1 are security backports only, with no API removal on the 4.x line — the breaking churn (grouped constant exports, MERGE_KEY removal, mandatory identify) is all on 5.x, which the `<5` ceiling already excludes. Verified by resolving js-yaml from @changesets/parse's own path (4.3.1, not a nested copy) and running `changeset status`, which parsed all 14 changesets and produced the correct per-package bumps. No cooldown bypass was needed: every target is 13-19 days old against the 7-day minimumReleaseAge. The comment claiming "4.3.0 is still inside the cooldown" was 54 days stale. None of the four is a runtime dependency of any published package, so nothing ships them and this needs no changeset. Also corrects dependabot.yml's "the only Rust in the repo" — there are two cargo entries, and the /packages/eql one landed with the subtree.
# Conflicts: # packages/eql/crates/eql-bindings/sql/release-manifest.json # packages/eql/package.json # packages/eql/packages/eql/sql/release-manifest.json # packages/eql/packages/eql/src/generated/release-manifest.ts # packages/eql/pnpm-lock.yaml
`biome.json` declared schema 2.5.2 while `package.json` has required `^2.5.3` and the lockfile has resolved 2.5.3 since before this branch — so `biome check` exits 1 with "The configuration schema version does not match the CLI version 2.5.3", and `code:check` fails for a reason unrelated to any code in the tree. It stayed hidden because node_modules on this machine still held 2.5.2; a `pnpm install` that brings the tree in line with its own lockfile is what surfaces it, which is exactly what CI does on every run.
The subtree sync brought upstream's actual 3.0.5 release, and it is not the bundle this branch had been carrying under that number. Upstream 142f41d restored `eql_v3.ste_vec_contains` as a deprecated delegating alias — both overloads — and retargeted its own changeset from `major` to `patch` in the same commit. The two decisions are one decision: it is a patch BECAUSE the old name still resolves. This branch had taken the other half of that trade — hard removal, still shipped as a patch — so the tree held SQL stamped 3.0.5 that differed from published 3.0.5 by 32 lines. That is the defect 9b1c44d exists to prevent, one level up, and nothing would have caught it: `verify-release-assets.mjs` compares the manifest's version to package.json and hashes nothing, and `sync-generated.mjs` preserves `src/generated/release-manifest.ts` rather than regenerating it, so `check:generated` is blind to it too. The merge landed on upstream's bytes exactly — every artefact here now hashes to `accde0030…`, the digest in the published tarball — so no regeneration was needed beyond re-emitting the two prisma migrations that bake the bundle. ONLY TWO MIGRATIONS WERE RE-EMITTED, deliberately. All four `migration.ts` files call `readVerifiedInstallSql()`, so re-running the 3.0.2 and 3.0.4 edges would silently bake 3.0.5 SQL into artefacts describing historical releases. Looping over the directory is the obvious thing to do and it is wrong; the frozen-digest test catches it, but only after the damage. The one assertion whose MEANING changed is `not.toContain('ste_vec_contains')`. Re-pinning it would have been wrong in both directions, so it now asserts the SHAPE the alias has to take: exactly two definitions, every body delegating to the new name rather than copying the implementation, and both marked deprecated with `COMMENT ON FUNCTION` so a DBA reading the schema sees it — not only the generated docs. Everything else that moved is prose that had inverted. The upgrade note was the worst of it: `v3.0.5.md` told readers to verify with a query commented "Must return zero rows" against `proname = 'ste_vec_contains'`, which now returns two — a reader following it would conclude the upgrade had failed. The release is now genuinely non-breaking, which leaves U-002 (every EQL install opens `DROP SCHEMA … CASCADE`) as the only item in it with operational consequences.
🦋 Changeset detectedLatest commit: e4ddc2d The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 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 |
This was referenced Aug 19, 2026
…annot serve
`refreshCargoLock` passed `--offline` on the reasoning that `eql-bindings`
resolves from a path and so needs no registry. The premise is true and the
conclusion does not follow: `cargo update -p X` does not update X in
isolation. It re-resolves the whole graph and rewrites a complete lock, and
offline that means every OTHER package has to be served from the local
registry cache. `packages/protect-ffi` has 167 of them.
The release job has no such cache. `jdx/mise-action` runs there with
`install: true, cache: false` — it installs toolchains and populates nothing
under `~/.cargo/registry` — and `lint-no-workflow-caching.mjs` forbids a
cache restore anywhere an artifact is published. Reproduced against this
tree with an empty CARGO_HOME:
error: no matching package named `chrono` found
location searched: crates.io index
required by package `protect-ffi v0.1.0 (…/crates/protect-ffi)`
exit 101. `execFileSync` throws, `pnpm run version` fails, and there is no
Version Packages PR — AFTER `changeset version` has already rewritten every
manifest and CHANGELOG, in a job holding `contents: write`. That is the same
half-applied release the mise-install step above it exists to prevent, and
the sync runs both workspaces before `release:prepare_bindings_assets`, the
only step that would have warmed anything, so the first call dies.
The measurement that made `--offline` look safe was taken on a developer
machine with a warm `~/.cargo`. Both resolutions were re-checked here from
cold and from warm: byte-identical, including the `windows-sys` edge cargo
repairs along the way. So this changes where the step can run, not what it
produces.
The old test asserted `--offline` was present, which pinned the defect
rather than catching it. It now asserts the flag's ABSENCE, paired with a
test of the CI precondition that absence depends on — no cargo cache, no
warm-up step — so that if someone ever warms the registry deliberately, the
coupling is visible instead of buried in a comment. The remediation advice
in `cargo-lock-freshness.test.mjs` recommended the same broken command and
is corrected with it.
`docs_v3_grep.sh`'s new check that deprecated `ste_vec_contains` aliases
never reach Doxygen was written as
elif tasks/docs/doxygen-filter.sh src/v3/json/functions.sql | grep -q …
under `set -euo pipefail`. On the LEAK path — the one the guard exists for —
`grep -q` matches, exits 0 and closes the pipe; the filter's awk takes
SIGPIPE and exits 141; `pipefail` makes that the pipeline's status, so the
`elif` is false and the FAIL branch never runs. Only the clean path (grep
reads to EOF, exits 1) was reported correctly.
This is not latent. Measured at exit 141 against the real ~16 KB filter
output on macOS, whose pipe buffer starts at 16 KB — grepping for a token
that IS in the output returned "no match". Linux's 64 KB buffer absorbs the
write and hides it, which is the same platform split
`workflow-grep-q-pipelines.test.mjs` already documents for GNU tar vs
bsdtar, and the same fail-OPEN direction it warns about.
Fixed by capturing first. The separate assignment also means `set -e` aborts
if the filter itself crashes, where the old pipeline scored a crashed filter
as a pass. Verified in both directions: with the `@cond` markers stripped so
the aliases genuinely leak, the guard now exits 1; before the fix it printed
OK either way.
The guard against the shape now covers tracked shell scripts, not just
`run:` blocks — the hazard is a property of the shell, not of GitHub
Actions. `echo`/`printf` writers are exempt: the whole write lands in the
pipe buffer before grep can exit, so there is no blocked writer to signal.
That exemption is about SIZE, not about them being builtins (a builtin in a
pipeline still runs in a subshell and still takes SIGPIPE), and it is what
keeps the thirteen bounded upstream call sites out of scope while leaving
the one real offender in it.
Whole-line comments are skipped, because the rule has to survive being
written down: the fix comment on `docs_v3_grep.sh` quotes the very pipeline
it replaced, and without that the guard reported the explanation as the
defect.
The fix is in vendored subtree code, so it needs to go upstream to
cipherstash/encrypt-query-language as well or the next sync will bring the
broken form back.
AGENTS.md asserted a live verdict — "It fires today on the hand-applied 3.0.5 bump … and it blocks the Version Packages PR as well as the publish" — which stopped being true the moment `@cipherstash/eql@3.0.5` reached npm. This branch's own 2a140e8 re-emits against the published 3.0.5, and `node scripts/release-gate.mjs` now exits 0 with "nothing to publish". A durable agent instruction that says to expect a red gate is worse than no instruction: it turns a real blocker into an expected one and the agent stops looking. The plan doc carried the same claim in weaker form. Both now state the CONDITION under which the gate fires and point at the script for what it blocks at any given moment — that answer is computed from the registry at run time and does not belong in a checked-in sentence. `frozen-publisher-docs.test.mjs` guards both halves of the drift. No doc may assert a live gate verdict; it went red on both files before this change. And each doc documents the eql freeze if and only if `FROZEN_PUBLISHERS` still carries it — keyed on the map rather than on a date, so the Phase-5 cutover cannot delete the entry without taking the prose with it. That is the same drift in the other direction, and it is already scheduled.
The 1.0.0 -> 3.0.5 upgrade workflow landed in `packages/stack-prisma`'s
README and in a changeset, but not in the skill. `skills/` ships inside the
`stash` tarball and `installSkills()` copies it into the user's
`.claude/skills/`, so the README is what a human reads after hitting the
error and the skill is what an AGENT reads before causing it — and an agent
driving `db init` in a repo upgraded from 1.0.0 met
Operation cipherstash.upgrade-eql-v3-bundle-3.0.5 has class "data"
which is not allowed by policy.
with no route out of it. The skill's install section still showed only the
first-time `contract emit` -> `migration plan` -> `migrate` sequence and
said nothing about upgrades, or about `migration plan` being the only
command that vendors new migration packages.
`stale-vendored-space.test.ts` already pinned the README to the planner's
real refusal string, on the reasoning that the message cannot be improved
from inside the package so the shipped docs have to bridge it to the remedy.
That reasoning covers the skill identically, so the assertion is extended to
both files rather than duplicated as a naked content check — the pair now
cannot drift from the message the planner actually emits. It went red on the
skill alone.
Adds the `stash` patch changeset a skills change requires, and widens the
skill's frontmatter trigger to the upgrade path.
coderdan
reviewed
Aug 19, 2026
Contributor
There was a problem hiding this comment.
Agent-assisted review (Claude Code, adversarially verified — refuted candidates dropped). Ten findings; three are inline below on the lines involved. The rest touch files or lines this PR doesn't change, so they're here:
1. Five workflows never trigger on packages/eql/crates/**. eql-bindings now compiles into index.node from the tree, but only integration-drizzle.yml and integration-protect-ffi.yml gained the trigger path. integration-supabase.yml, integration-prisma-next.yml, prisma-next-e2e.yml, prisma-example-readme-e2e.yml (all run build-ffi-binding) and tests-rust.yml (cargo-tests the dependent crate) still watch only packages/protect-ffi paths — and wasm-build-inputs-paths-filter.test.mjs scans only wasm: 'true' callers, so the native-only workflows escape the guard. A payload-encoding change under packages/eql/crates/ merges with those suites green-by-absence.
2. The @cipherstash/eql runtime pin loosened from exact 3.0.4 to workspace:^, which packs as ^3.0.5 in the published tarballs — while npm publishing rights for that package still sit with the old repo until the cutover. A 3.0.6 published from there would be resolved by a registry install of stash and applied by stash eql install with no digest check (readInstallSql is trusted verbatim; readVerifiedInstallSql verifies against the resolved package's own manifest, which a substituted package rewrites too). That's the emit/store skew this absorption exists to prevent, reintroduced through the packed-tarball path. The exact-pin rationale deleted from migration-v3.test.ts isn't re-established anywhere.
3. skills/stash-prisma/SKILL.md doesn't know about the re-emitted baseline or the upgrade edge. The README and changesets document the "run migration plan first, or db init fails with the raw policy-refusal error / migrate silently stays on 3.0.4" flow — but the skill, which is the artifact that actually lands in customer repos to answer exactly this, is untouched. Per AGENTS.md, a Prisma Next integration change checks that skill in the same PR.
4. skills/stash-postgres/SKILL.md:165 still claims "The CLI pins an exact version, so a database is only ever on one bundle" — no longer true after the workspace:^ change, and this branch's own changeset says the claim was dropped (the diff removed only the other copy). Several lines in that skill also still point at the old repo as where the bundle is developed, contradicting this PR's SECURITY.md note.
5. Missing changeset for protect-ffi's eql-bindings move (crates.io =3.0.2 → in-tree path dep at 3.0.5) — that changes what compiles into the published binding. Since main has now executed the changeset cutover, on rebase this lands as a regular .md, not a parked one.
Also worth noting: the first inline finding below (the upgrade edge cascade-dropping customer functional indexes) is tracked generally as #918 — this PR's edge is the first shipped instance of it.
Two more on files in this PR, but on lines that pre-date its hunks:
6. scripts/sync-lockstep-versions.mjs:206 — fragile main-guard. Fragile main-guard: import.meta.url percent-encodes, process.argv[1] doesn't, so on any checkout path containing a space (or other URL-encodable character) this comparison is false and the script silently no-ops with exit 0 — changeset version applies but the Cargo.toml bump, Cargo.lock refresh, and SQL-asset regen are all skipped. That's exactly the silent lockstep skew the script exists to prevent, and the SQL-asset half isn't caught by cargo-lock-freshness.test.mjs.
This same PR already has the robust form at lint-no-eql-registry-pins.mjs:733: process.argv[1] === fileURLToPath(import.meta.url).
7. .github/workflows/macro-expand-eql.yml:71 — rust-cache points at the wrong workspace. workspaces: packages/eql/tests/sqlx points at a workspace member (it's in the root Cargo.toml's members, has no Cargo.lock of its own), so rust-cache hashes no lockfile and caches a target/ directory cargo never writes — artifacts land in packages/eql/target. Every nightly run cold-compiles the full SQLx dependency tree inside the 30-minute timeout while restoring and saving a useless cache. Should be workspaces: packages/eql like the sibling jobs.
Two knock-ons: this step also omits save-if: false, breaking test-eql.yml's invariant that only build-archive saves the shared sqlx-tests key — and the eql-suite-ci guard only scans test-eql.yml, so this copy escapes it. (Same fix pattern applies to test-eql.yml's splinter job, which has no rust-cache restore at all.)
| @@ -76,9 +77,10 @@ export default class M extends Migration { | |||
| sql: "SELECT to_regtype('eql_v3.query_json') IS NOT NULL", | |||
Contributor
There was a problem hiding this comment.
This upgrade edge re-runs the full install SQL, whose first statement is DROP SCHEMA IF EXISTS eql_v3 CASCADE — and the CASCADE takes every customer object built on eql_v3 functions with it, most importantly the functional indexes skills/stash-prisma tells customers to create. Those were created by a one-shot migration that never re-runs, so nothing recreates them: precheck is [], the postchecks assert only function/domain existence, the migration reports success, and every encrypted predicate silently degrades to a sequential scan.
The branch's changeset concedes the mechanism ("this is the same mechanism that drops functional indexes") but then says grants are the only part of the upgrade needing action — for a GA database with indexes, that's not true. General tracking issue: #918 (capture pg_get_indexdef() before the drop, re-issue + ANALYZE after). At minimum this edge should warn, or the changeset should stop saying no action is needed.
| continue | ||
| } | ||
| if ( | ||
| versionsOf(dep.name).some((candidate) => satisfies(candidate, range)) |
Contributor
There was a problem hiding this comment.
Ordering bug with a dependency-confusion consequence: the registry-satisfaction check runs before the target.private check, so a private workspace dependency whose name is squatted on npm at a satisfying version is continued past — the private-dependency blocker never fires. Verified by direct execution: squatted name → [], absent name → blocker. That contradicts this function's own doc comment ("a private dep is a blocker either way").
Scenario: a published package gains a runtime workspace:* dep on a private member whose name exists on npm; packedRange rewrites it to a registry range at pack time, the gate exits 0, and customer installs resolve the squatter's package. Fix: hoist the target.private check above the registry check.
| 'There are exactly two ways past this, and neither is editing this gate:\n\n' + | ||
| ' 1. Publish the frozen package. For @cipherstash/eql that is the Phase 5\n' + | ||
| ' cutover in docs/plans/2026-08-13-eql-monorepo-absorption.md: repoint\n' + | ||
| ' npm trusted publishing to cipherstash/stack, release 3.0.5, and every\n' + |
Contributor
There was a problem hiding this comment.
Already stale before merge: @cipherstash/eql@3.0.5 is on npm now, so the gate exits 0 today and this remediation text ("release 3.0.5, and every finding above clears") describes a registry state that no longer holds — as do the FROZEN_PUBLISHERS reason at line 131 ("npm's newest is 3.0.4"), the tests.yml comment, and AGENTS.md's "It fires today on the hand-applied 3.0.5 bump". Next time the blocker actually fires (say a 3.0.6 pre-cutover), the message tells the operator to release a version that's already out. Suggest deriving the version from the blocker data and rewording the "fires today" prose to past tense.
coderdan
requested changes
Aug 19, 2026
coderdan
left a comment
coderdan
left a comment
Contributor
There was a problem hiding this comment.
Recommend addressing Claude's findings.
Audit of the #885 review (2026-08-14) and the #915 review (2026-08-19) against the current tree, plus fixes for what was still open. Most of both was already addressed by 4098a1f..00d685f and the uncommitted work this commit also carries; what follows is only what was not. STALE — no longer true, verified rather than assumed: * "eql_v3.ste_vec_contains is hard-removed (no alias)". Upstream restored the deprecated delegating aliases in 142f41d and 3.0.5 shipped with them; the subtree sync 88c21c9 brought that here and 2a140e8 re-emitted against it. * "protect-ffi's Cargo.lock is stale at 3.0.4". Both locks read 3.0.5, and cargo-lock-freshness.test.mjs holds them there. * All four blockers, which landed in ca2b0a7 / 4098a1f with guards. REJECTED — the recommendation would make things worse: * "The re-emitted baseline keeps createdAt: 2026-07-14 — misleading provenance." createdAt is not provenance: it is the PRIMARY TIE-BREAK KEY in the migrator's neighbour ordering (createdAt -> to -> migrationHash), so a re-emit of the same logical migration must not shift its position in the graph. It is also hashed, so moving it would have surfaced as "re-pin this digest" and slipped through. Pinned explicitly in migration-v3.test.ts, with the reason written into the migration header. * "path filters miss packages/eql/docker/**". Its only CI consumer is test:docs_v3_grep reading docker/README.md, in the docs-static job — which carries no relevance gate and so runs on every pull request already. FIXED: * 7a9604c moved eql-bindings from crates.io =3.0.2 to an in-tree path dep at 3.0.5 — a change to what compiles into the published binding — and shipped with no changeset. Added, as a patch: the crate's Rust is byte-identical across 3.0.2/3.0.4/3.0.5 (only its bundled SQL moved), so emitted payloads do not change; what changes is that the emitter and the SQL are now the same commit and the skew is unrepresentable. * test-eql.yml's `splinter` job compiled Rust uncached. It looks Rust-free — its task is bash over psql — but its build step runs `mise run build`, and tasks/build.sh shells out to `cargo run -q -p eql-codegen` twice. New invariant in eql-suite-ci.test.mjs: every job invoking a mise task that REACHES cargo must restore the shared cache. It closes over the task graph rather than grepping the workflow, because the string `cargo` appears nowhere in it — and it found exactly this one job. * Two `.changeset/*.md.deferred` files were still sitting in this tree, and the obvious reading of them is wrong. The cutover did its job properly: e77bfce retired the guard AND renamed both files in one commit, and they released in @cipherstash/protect-ffi@0.32.0. This branch was cut before that, so it still carries them under the old suffix — where reactivating them would republish a shipped changelog entry and re-bump the package for a change two versions old. Deleted, not renamed. New guard scripts/__tests__/no-parked-changesets.test.mjs fails on the suffix either way and points the reader at the released CHANGELOG to decide which resolution is right. It also fails if the retired guard is reinstated beside it, since a half-retired convention is what produces a parked file. The retired guard, its self-test, its fixtures, the lint:ffi-changeset script and the tests.yml step are deleted here too, matching main. * packages/protect-ffi/README.md still taught the embargo and the .md.deferred parking, both over. It ships in the tarball, so the wrong instructions ship with it. AGENTS.md and the Phase-4 checklist corrected likewise. Also carries the previously uncommitted work for #915's findings 1-4, 6 and 7: the five workflows missing packages/eql/crates/** triggers, the exact runtime pin (workspace:^ -> workspace:*) with frozen-publisher-runtime-pins.test.mjs, the two skills corrections, the robust main-guard form with script-main-guards.test.mjs, and macro-expand-eql.yml's rust-cache workspace. Suites: 683 script tests, 363 stack-prisma, protect-ffi JS green; release-gate exits 0; lint:eql-pins clean; `changeset status` resolves with the seven FFI packages at patch.
Brings #885's merge of main (8e9e10a) down into this branch so the stack can land as a unit. Fourteen conflicts; none resolved by picking a side blindly, and three needed the artefacts regenerated rather than chosen. REGENERATED, NOT PICKED. Both sides re-emitted the baseline and the 3.0.5 upgrade edge, and both hashes were wrong for the merged tree: this branch's bake the published 3.0.5 SQL under the pre-0.17 `sha256:`-prefixed hash, #885's bake 3.0.4-era SQL under 0.17's bare one. The right answer is neither — it is this branch's BYTES under #885's HASH FUNCTION. Re-ran each migration's own self-emit (`tsx migrations/<dir>/migration.ts`) and re-pinned `migration-v3.test.ts` to what came out: baseline bad30c9b… (was sha256:23c98b03… / 9447442333…) 3.0.5 upgrade edge 8c47bd1d… (was sha256:3b2b838b… / 4050dee89e…) `ops.json` is unchanged by the re-emit in both: `installSqlSha256` stays accde0030…, byte-identical to the published `@cipherstash/eql@3.0.5` tarball (verified by `npm pack`), and `createdAt` is preserved — it is the primary tie-break key in the migrator's neighbour ordering, not provenance. The 3.0.2 and 3.0.4 edges were NOT re-emitted and still bake 05860ae4… and 63104a81…, their own releases: `readVerifiedInstallSql()` reads the CURRENTLY installed bundle, so re-emitting a historical edge would bake today's SQL into it. THE 0.17 SCOPE RENAME BROKE TWO FILES THAT NOTHING WOULD HAVE CAUGHT TOGETHER. main's c795225 retired the whole `@prisma-next/*` scope for `@prisma/orm-*`. * `stale-vendored-space.test.ts` (added here in 810b069) imported six modules from it — a hard failure, so it announced itself. Ported, plus the two API changes behind it: `emitContractSpaceArtefacts` → `…Artifacts`, and `readMigrationsDir` now requires `{ migrationsDir }`. Its fixture then failed for a REAL reason worth keeping. It pinned the hash 1.0.0 published (`sha256:fc495f7f…`); under 0.17 the same bytes hash to `1030654387…` — which is exactly what `@cipherstash/stack-prisma@1.1.0` ships, so the fixture is re-pinned to a published artefact rather than a computed one. The stale-but-SILENT case is now 1.1.0's, and a new case pins 1.0.0's: its prefixed digest cannot equal a bare recomputation, so `readMigrationsDir` reports `hashMismatch` and that space IS caught. The suite covers both vendored generations; 16 tests, all green. * `skills/stash-prisma/SKILL.md` took a section from this branch that rides `rawSql` imported from `@prisma-next/postgres/migration` — two dead package names in the file whose whole job is telling a customer what to type, and nothing compiles a skill. 0.17 also replaced that recipe with PSL expression indexes, so the section was redundant as well as wrong. Kept only what main's version lacks — the EQL-upgrade-drops-your-indexes warning — rewritten against the surface that ships, and the changeset with it. New guard `scripts/__tests__/skills-retired-package-scopes.test.mjs`: no `skills/*/SKILL.md` may name a member of a retired scope. The scope WILDCARD stays legal, because `stash-prisma` has to be able to say the scope is retired — so the rule is `@prisma-next/<pkg>`, never `@prisma-next/*`. Verified against the defect: reinstate the line and it fails naming file, line and specifier. CORRECTED WHILE RESOLVING, both verified against the registry rather than reasoned about: * AGENTS.md and `packages/protect-ffi/README.md` both said no FFI release had run from this repository and to "treat the path as configured rather than proven". False since 0.32.0: its SLSA provenance names github.com/cipherstash/stack and .github/workflows/release.yml. Written while 0.31.0 was newest and never revisited. Both now carry the fact and the one-line curl that re-checks it. * The plan doc's Phase-4 note warned that #905 had to rename its parked changeset before merging. It merged as f7abfec without doing so, so `.changeset/protect-ffi-jsonwebtoken-cve.md.deferred` is on main now — a protect-ffi patch for CVE-2026-25537 that `@changesets/read` cannot see. Committed version and npm newest are both 0.32.0, so it is genuinely unreleased and the fix there is `git mv` back to `.md`. Recorded, not fixed: this branch predates #905 and does not carry the file. Other resolutions: pnpm-workspace.yaml keeps this branch's four re-selected security overrides AND main's `sharp@<0.35.0` (disjoint sets); the lockfile was regenerated rather than hand-merged and `--frozen-lockfile` passes; the `release:gate` step in tests.yml and the corrected `.md.deferred` paragraph in AGENTS.md are this branch's; the 1.1.0 mention in the 3.0.5 changeset is main's. Root package.json's `lint:package-paths` line lost the stray two-space indent 2e98298 gave it — a `code:check` error on this branch as it stood. Suites: 688 script tests, 364 stack-prisma (16 in the ported suite), 1344 CLI, 94 protect-ffi JS. `biome check` error-free, `release-gate` exits 0, `lint:eql-pins` clean, `changeset status` resolves with the seven FFI packages at patch and no bump for the frozen `@cipherstash/eql`.
The #885 review found, by reading a diff, that this branch's in-tree `@cipherstash/eql@3.0.5` was not the published 3.0.5: the subtree's install bundle hashed `7ad9c9f8…` while npm's carried `accde0030…`, because upstream had restored the deprecated `ste_vec_contains` aliases in the actual release. The subtree sync fixed the bytes. Nothing was added to stop it recurring, and it is a whole class rather than one slip. WHY NOTHING IN THE TREE COULD SEE IT. `sql/release-manifest.json` is regenerated alongside the SQL, so it agrees with whatever was generated — every digest verifies, `migration-v3.test.ts`'s lockstep check passes, `lint:eql-pins` is clean. The manifest is the artefact's statement about itself, and self-consistency is exactly what a skew preserves. Only the registry disagreed, and nothing was asking it. WHY IT ONLY APPLIES TO A FROZEN PACKAGE. For one this repo publishes, in-tree bytes differing from npm is an unreleased change — the normal state of every pull request, and blocking it would freeze the repository. For a frozen one it is a contradiction: the version cannot be released from here, so the tree is not PROPOSING those bytes, it is asserting they are already on npm under that number. That makes the comparison meaningful for exactly the set `FROZEN_PUBLISHERS` already names, and meaningless outside it. The reach is not theoretical: `packages/cli`'s installer calls `readInstallSql()` with no digest check, so `stash eql install` would have put `jsonb_document_contains` into a customer database under a version whose published SQL defines `ste_vec_contains` — two databases both truthfully reporting 3.0.5 and disagreeing on the function set. CHECK C, in `scripts/release-gate.mjs` beside the two it already runs. For each frozen package it `npm pack`s the committed version and compares the published release manifest's digest with the in-tree one. Silent in three cases, each tested: a package this repo can publish; a version npm does not carry (that is `publishBlockers`' `frozen-publisher`, and two blockers for one fact make the remedy ambiguous); a private manifest. It THROWS for a frozen publisher with no artefact declared — a check with nothing to compare reads exactly like a check that passed, which is the shape `lint-no-eql-registry-pins.mjs` exits 2 on. `FROZEN_ARTEFACT_DIGESTS` holds the artefact paths, keyed identically to `FROZEN_PUBLISHERS` and asserted so by equality, so a frozen publisher cannot arrive without one. THE REMEDIATION HAD TO BE WRITTEN SEPARATELY, and this is the part worth reading. `reportBlockers`' first way out is "publish the frozen package" — which for a bytes skew is precisely wrong: the version IS on npm, that is how the skew was detected. An operator following it would try to release something already released. The skew branch is checked first, names both digests, and says the tree is what moves. Costs one `npm pack` per frozen package (today: one) on a job that already talks to the registry. `npm` and `tar` rather than a gzip+tar reader written here — both are on every runner image and npm was already this script's one external command, while a hand-rolled parser would be parsing code inside the script whose job is to be trusted. Extraction is scoped to the single manifest path. Verified end to end against the live registry, not just the injected-reader unit tests: rewind the in-tree digest to `7ad9c9f8…` — the value this branch actually shipped — and the gate exits 1 naming both hashes. 695 script tests green; `biome check` error-free; the gate exits 0 on the tree as it stands.
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
3 participants
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.
Stacked on #885 — review that first, or review this alone as "the four blockers found reviewing #885, plus the upstream 3.0.5 sync."
ea9e1401..HEAD: 80 files, ~7.7k insertions (excludingpackages/eql, which moves only via the subtree merge below).The four blockers
1 — the release job could not run the version hook it now depends on.
release.ymlpassesversion: pnpm run version, which chainssync-lockstep-versions.mjs→mise run release:prepare_bindings_assets→cargo run -p eql-codegen. The job installed pnpm, Node and node-gyp; mise is not on GitHub's ubuntu images..changeset/already held live changesets, so changesets/action takes its version branch on the next push to main regardless of this PR —pnpm run versiondies with ENOENT and no Version Packages PR is opened at all.release-version-hook.test.mjsasserted the wiring existed; nothing asserted the job could execute it.add_shims_to_path: falseis load-bearing and would not survive a copy-paste fromtest-eql.yml: mise's shim dir is prepended to PATH for every later step, so its pinned Node 22 would shadow setup-node's andchangeset publishwould shell out to npm 10.x — below the 11.5.1 OIDC trusted publishing requires.2 — merging armed a release that could not succeed.
changeset publishpublishes every public workspace package whose version is absent from npm — changeset or no changeset — withPromise.all, no dependency ordering, andpublishAPackagereturning failures rather than throwing them. So a package that cannot publish does not stop the siblings that depend on it.publishBlockersinrelease-gate.mjsanswers that before anything publishes and exits non-zero, failinggateand skippingrelease.FROZEN_PUBLISHERShas to be written down rather than derived: npm exposes no way to ask "would a publish from here be accepted?", and provenance on the last release is evidence about the past, not permission for the next.3 — the cache keys could not see the path dep. Phase 3 made
eql-bindingsa path dependency ofprotect-ffi, so it is a compile input toindex.node— but both keys hashed onlypackages/protect-ffi/**. A src-only edit there changes no hashed file, so the cache hits and every credentialed job runs a stale binding. This PR's own 3.0.4 → 4.0.0 → 3.0.5 bumps moved neither key.4 — the PG 14–16 matrix ran never.
test-eql.ymlgated the full matrix behindmerge_groupalone. Checked against the live API:mergeQueue(branch:"main")is null,branches/main/protection404s, and the active ruleset carries norequired_status_checksrule — soci-requiredis required by nothing andmerge_groupis an event this repo never emits.setupis now an exclusion, so the full matrix is the default and a trigger added later gets more coverage rather than none.The upstream 3.0.5 sync
@cipherstash/eql@3.0.5published fromcipherstash/encrypt-query-languagewhile this branch was open — and it is not the bundle this branch carried under that number. Upstream142f41d8restoredeql_v3.ste_vec_containsas a deprecated delegating alias (both overloads) and retargeted its changeset frommajortopatchin the same commit. Those are one decision: it is a patch because the old name still resolves.The subtree is re-synced to the release commit; every artefact here now hashes to
accde0030…, byte-identical to the published tarball. Nothing would have caught the divergence —verify-release-assets.mjscompares versions and hashes nothing, andsync-generated.mjspreservessrc/generated/release-manifest.tsrather than regenerating it, socheck:generatedis blind to it.Only two prisma migrations were re-emitted, deliberately. All four
migration.tsfiles callreadVerifiedInstallSql(), so re-running the 3.0.2 and 3.0.4 edges would silently bake 3.0.5 SQL into artefacts describing historical releases. Looping over the directory is the obvious move and it is wrong.Also here
bench-eql.ymlpassed all 14 tests. That case is now a permanent test with the workflow list injected rather than the file deleted.pnpm-workspace.yaml— where pnpm actually resolvesoverridesandcatalogs, and the quietest way to reintroduce the skew the whole absorption exists to prevent.migrations/cipherstash/is never detected (11 new tests).migrationHashverifies a package against its own bytes, so a 1.0.0 copy is stale but internally intact and passes everything. On a fresh databasedb initthen refuses with an error naming neither the directory nor the remedy.SECURITY.mdrow.Not in this PR
The security-override refresh and the Biome config-schema fix are independent of EQL and target
maindirectly — see the linked PRs. Both also appear in this branch's history; they will resolve whenmainmerges in.Second review round (2026-08-19)
Every item was checked against the tree rather than taken at face value; three did not survive that check.
Stale — fixed upstream while the review was open.
eql_v3.ste_vec_containswas never hard-removed in the end: upstream142f41d8restored both deprecated overloads and 3.0.5 shipped with them, which is also why its changeset is apatch. And bothCargo.locks already read 3.0.5, held there bycargo-lock-freshness.test.mjs.Rejected —
createdAton the re-emitted baseline. The ask was to move it forward to match the new bytes. It is not provenance: it is the primary tie-break key in the migrator's neighbour ordering (createdAt → to → migrationHash, seefindPath/findPathWithInvariants), so re-emitting the same logical migration must not shift its position in the graph. It is also hashed intomigrationHash, which is why moving it would have surfaced as "re-pin this digest" and gone through unremarked. Now pinned explicitly inmigration-v3.test.ts— mutation-checked, and it fails with a message about path ordering rather than digests — with the reasoning in the migration header where the question gets asked.Rejected —
packages/eql/docker/**in the path filters. Its only CI consumer istest:docs_v3_grepreadingdocker/README.md, in thedocs-staticjob, which carries no relevance gate and therefore already runs on every pull request. Adding it would only fire the full PG matrix on docs pushes tomain, against that file's own documented cost reasoning.Fixed — the
eql-bindingsmove had no changeset.7a9604c6took it from crates.io=3.0.2to an in-tree path dep at 3.0.5, which changes what compiles into the published binding. Added as apatch: the crate's Rust is byte-identical across 3.0.2/3.0.4/3.0.5 (only its bundled SQL moved), so emitted payloads are unchanged — what changes is that the emitter and the SQL are now the same commit.Fixed —
splintercompiled Rust uncached. The job looks Rust-free (its task is bash over psql), but its build step runsmise run build, andtasks/build.shshells out tocargo run -q -p eql-codegentwice. New invariant ineql-suite-ci.test.mjs: every job invoking a mise task that reaches cargo must restore the shared cache. It closes over the mise task graph rather than grepping the workflow — the stringcargoappears nowhere intest-eql.yml's splinter job — and independently found exactly that one job.Fixed — two
.md.deferredchangesets, where the obvious reading is the wrong one. The cutover did its job properly:e77bfcecretired the guard and renamed both parked files in one commit, and they released in@cipherstash/protect-ffi@0.32.0. This branch was cut before that, so it still carried them under the old suffix — where "activate the parked changeset" republishes a shipped changelog entry and re-bumps the package for a change two versions old. Deleted, not renamed. New guardno-parked-changesets.test.mjsfails on the suffix either way and points the reader at the released CHANGELOG to decide which resolution is right; it also fails if the retired guard is reinstated beside it, since a half-retired convention is what produces a parked file.packages/protect-ffi/README.mdstill taught the embargo and the parking, and it ships in the tarball — corrected, along with AGENTS.md and the Phase-4 checklist.The remaining findings (the five workflows missing
packages/eql/crates/**triggers, theworkspace:^→workspace:*runtime pin withfrozen-publisher-runtime-pins.test.mjs, the two skills corrections, the robust main-guard form, andmacro-expand-eql.yml's rust-cache workspace) are in2e98298dand the commits before it.