fix(cli): make EQL reinstall dependency-safe

[tobyhede]

Summary

EQL upgrades replace eql_v3 and eql_v3_internal with DROP SCHEMA ... CASCADE. Encrypted columns and rows survive, but customer-created objects that depend on EQL machinery may not. This PR makes stash eql install and stash eql upgrade preserve reconstructable search indexes and refuse before mutation when a dependency cannot be restored safely.

Changes

Dependency-safe reinstall

• Acquire a bounded advisory lifecycle lock so concurrent lifecycle commands queue and an abandoned holder eventually produces an actionable error.
• Walk PostgreSQL dependencies transitively, including rewrite-rule-to-view edges.
• Classify installed EQL operators and function-backed casts from the installed catalog rather than the incoming bundle.
• Capture ordinary functional indexes, replace the EQL schemas, rebuild the indexes, and verify definition, validity, readiness, clustering, replica identity, comments, and per-column statistics targets. Index ownership follows the table owner in PostgreSQL and is verified rather than separately restored.
• Refuse unsupported dependencies—including views, policies, constraints, generated columns, triggers, customer operators, and partitioned index trees—before destructive SQL.
• Run capture, replacement, reconstruction, verification, and rollback protection in one transaction.

The advisory lock coordinates stash lifecycle commands, not arbitrary application DDL, so a maintenance window is still required.

Assessment and command behavior

• Centralize installed-EQL assessment while preserving advisory permission and ORE reporting.
• Derive capability schema presence from the same catalog observation as installation state.
• Preserve caller transactions and original errors around surface/version savepoints.
• Render typed reinstall refusals as expected command failures rather than fatal crashes.

Live verification

• Forward STASH_TEST_DATABASE_URL through Turbo's strict environment contract.
• Serialize live suites that share the disposable EQL schemas.
• Cover rollback after verification failure, malformed capture metadata, installed-catalog cast ownership, index owner/statistics fidelity, and released-version encrypted-index upgrade.
• Exercise the CLI live suite against PostgreSQL 16 and 17 while retaining both supported Node.js test legs.
• Load protect-ffi dynamically inside the credential-gated upgrade suite, so ordinary test collection does not require its native artifact.

SET LOCAL jit = off applies only within the lifecycle transaction.

Verification

• Full CLI unit suite: 1,495 passed.
• Full local Postgres suite: 1,528 passed; 7 credential-dependent tests skipped locally.
• Script contract suite: 946 passed.
• CLI build and repository code:check pass.
• EQL SQLx archive and all four PostgreSQL 17 shards pass in CI.

Scope

The agent-workflow and domain-documentation conventions are extracted to base
PR #962. This PR records the durable safe-reinstall decision in
docs/adr/0001-eql-data-survives-disposable-schema-reinstall.md; the obsolete
Superpowers specification is not part of the stack.

Commit structure

• fix(cli): make EQL reinstall dependency-safe
• test(cli): upgrade a released EQL bundle with real ciphertext
• refactor(cli): deepen EQL index restoration
• refactor(cli): deepen verified EQL bundle
• refactor(cli): deepen installer test scenarios
• refactor(cli): deepen EQL installation assessment
• refactor(cli): deepen EQL surface verification
• refactor(cli): deepen Supabase EQL access policy
• refactor(cli): deepen declared schema validation
• fix(cli): preserve EQL assessment and index state
• fix(cli): address safe reinstall review feedback

Related

Closes #918
Closes #959

Review guide

Start with packages/cli/src/installer/derived-search-index-restoration.ts. LIFECYCLE_DEPENDENCIES_SQL defines the safety boundary and the module owns capture, replacement, reconstruction, and verification ordering.

Then review packages/cli/src/installer/installation-state.ts for assessment semantics and turbo.json plus packages/cli/vitest.config.ts for the live-test execution contract.

[changeset-bot]

🦋 Changeset detected

Latest commit: 3867928

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
stash	Minor
@cipherstash/basic-example	Patch
@cipherstash/e2e	Patch
@cipherstash/stack-drizzle	Minor
@cipherstash/stack-prisma	Minor
@cipherstash/stack-supabase	Minor
@cipherstash/stack	Minor
@cipherstash/wizard	Minor
@cipherstash/bench	Patch
@cipherstash/prisma-example	Patch
@cipherstash/test-kit	Patch

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

[freshtonic]

Requesting changes on one blocker; the core engineering is genuinely good and the adversarial checks below came back clean. But this PR's safety story for a destructive customer-database path rests on live suites that CI silently skips — and the PR body asserts the opposite.

Blocker: the new live suites run in no CI job

I verified both halves directly:

• This PR's own Node 22 job log shows reinstall.live.test.ts (16 tests | 16 skipped) and upgrade-encrypted-indexes.live.test.ts (1 test | 1 skipped) — the refusal matrix, partitioned trees, lock behaviour, rollback-after-DROP, and the credentialed 3.0.2→current upgrade have run only on the author's machine.
• Mechanism: the workflow step sets STASH_TEST_DATABASE_URL, but pnpm run test goes through turbo, and turbo runs tasks in strict env mode — turbo.json's test task declares env: ["STASH_POSTHOG_KEY"] only, no globalEnv/passThroughEnv, so the variable is stripped before vitest sees it and describeLive falls to describe.skip. (Corroboration: packages/stack's live suite that reads credentials from a dotenv file did run in the same step — dotenv is immune to env filtering; every process.env.STASH_TEST_DATABASE_URL-gated suite skipped.)

The wiring gap is pre-existing (the same suites skip on main), but this is the PR that ships DROP SCHEMA ... CASCADE orchestration against customer databases with these suites as its only proof, extends the tests.yml comment claiming they run, and states in the body that "its complete execution is owned by the existing credentialed CI job". Fix is small: add STASH_TEST_DATABASE_URL to the test task's env (or passThroughEnv), then confirm from the job log that the suites execute — this also revives the pre-existing verify.live / applied.live / preflight.live / guarded-grants.live suites, whose silent skipping is exactly the "phantom damage on green CI" the workflow comment warns about. They'll add real minutes ×3 matrix legs (~15 installs at 10–30s each); gating the live suites to one leg is a reasonable trade to make deliberately.

Also fix before merge (small)

• PR body staleness: the review guide says "start with packages/cli/src/installer/reinstall.ts" (no such file — it's derived-search-index-restoration.ts); the commit-structure description doesn't match the actual two commits; and "disables JIT on its dedicated connection" — the code is SET LOCAL jit = off, transaction-scoped (stricter than the body claims, but say what it does).
• Stale lock-budget comment: reinstall.live.test.ts says "the production default is deliberately a minute"; LOCK_WAIT_MS is 300 000 and the changeset correctly says five minutes.

Please split out: the agent-workflow scaffolding

CONTEXT-MAP.md, docs/agents/domain.md / issue-tracker.md / triage-labels.md (wayfinding labels, sub-issue protocols), and the CLAUDE.md conventions section are new repo-wide conventions with no connection to EQL reinstall, introduced inside a CLI bugfix — including docs/adr/ as a new convention mandated by a doc that arrives in the same diff (repo precedent is docs/plans/). If the team wants these, they deserve their own PR and a deliberate decision, not convention-by-fait-accompli. (packages/eql/CONTEXT.md's glossary genuinely supports this fix and can stay; the biome.json schema-string bump and the stack-supabase test existsSync hardening are harmless smuggles — note or drop.)

Comments, no change demanded

• Cross-version refusal risk (plausible): the operator/cast ownership exemption is derived from the pinned bundle, but at upgrade time the database carries the old bundle's operators. A future release that renames a public operator would make upgrade classify the old bundle's own object as customer-owned and refuse a healthy database — fail-closed, so safe, but the message would blame the customer. The 3.0.2-baseline live test is exactly the guard for this, which loops back to the blocker: it currently runs nowhere.
• Local-dev collection coupling: upgrade-encrypted-indexes.live.test.ts imports @cipherstash/protect-ffi at module top, so even self-skipped, an unbuilt protect-ffi fails pnpm --filter stash test collection locally — same class of coupling packages/cli/AGENTS.md already documents.
• The Test EQL sqlx matrix legs (which execute the modified v3_uninstall_tests.rs) were still queuing at review time — confirm they finish green.

Adversarial checks that came back clean

SQL generation is injection-clean (every executed identifier is server-quoted via format('%I…') or verbatim pg_get_indexdef(); lock key and exemption lists are bind parameters). The advisory lock is transaction-scoped so it releases on every error path including connection death, the polling design avoids both the infinite-hang and lock-queue-deadlock classes, and the bounded-wait/queued trade is tested in both directions. The pg_depend classifier is genuinely fail-closed — matviews, extended statistics, publications, exclusion constraints, generated columns, unknown classes all refuse; the rewrite-rule→view hop is correctly scoped; partitioned parents and attached children both refuse; FK-referencing-expression-index is not a real gap in Postgres. Rebuild fidelity captures and verifies on one session/search_path with opclasses, INCLUDE, storage params, WHERE, UNIQUE and tablespace riding the server-rendered definition, plus clustered/replident/comment restoration. The one-DDL-transaction claim is real and the non-CONCURRENT lock consequence is honestly documented everywhere it matters (changeset, ADR, README, skill), as is the capture→drop TOCTOU window. No command/flag changes, so the manifest check holds; changeset is present, right package, right level, accurate.

Fix the turbo env wiring, show one CI log line of the suites actually executing, tidy the body, and this is an enthusiastic approve.

[cipherstash-bot]

Test-coverage review

The reinstall/restoration machinery is well covered on its happy paths (installer.test.ts drives the full capture→replace→reconstruct→verify sequence, and the live suites exercise partitioned-tree refusal, lock timeout, and DROP-SCHEMA rollback). Three negative/branch paths in the new code have no unit or live coverage — each is a failure the code explicitly claims to catch, so each deserves a test that watches it fire.

Inline comments below (3). Nothing crypto/security-related flagged.

The RecordingRestorationDatabase helper (__tests__/restoration-scenarios.ts) currently mirrors captured rows back on verify and only emits well-formed index rows on capture, which is why gaps 1 and 2 exist — both need a small option added to that harness to inject a divergent/malformed row.

---

⚠️ 1 review job(s) failed and were excluded from this review: codex (gpt-5.5)/test-gap.

[coderdan]

Review notes

Reviewed 4602dd49..f98b0288. The catalogue-walk SQL holds up: the bundle really does open with DROP SCHEMA … CASCADE, the public.eql_v3_* domains are IF NOT EXISTS-guarded so they survive, no two bundle operators share a (name, leftarg, rightarg) triple (so the count(*) = 1 uniqueness guard can't mis-fire), and non-index dependants (constraints, pg_attrdef, policies, rules→views) all fall through to the refusal path rather than being silently dropped. The approach is right.

Two things I'd want addressed before merge, then some smaller ones.

1. The refusal path is unhandled at the call site — medium

packages/cli/src/commands/db/install.ts:202

s.start('Installing EQL v3 extensions (pinned bundle)...')
await installer.install({ supabase })

install() now throws EqlReinstallRefusalError — that's the whole point of the PR — and nothing catches it. bin/main.ts:546 rethrows to the bootstrap "Fatal error" handler.

Concrete case: a Supabase user with an RLS policy calling eql_v3.eq_term runs stash eql install --force. The carefully-worded refusal listing each blocking object renders as a fatal crash under a spinner that never stops. The designed, expected outcome of this feature presents as a bug to the person it's protecting.

2. 'incoherent-mixed' is not a real OreSurfaceState — medium

packages/cli/src/installer/installation-state.ts:29 declares:

state:
  | 'indexable'
  | 'fallback'
  | 'incoherent-mixed'
  | 'incoherent-poisoned'
  | 'incoherent-unpoisoned'

OreSurfaceState (installer/ore.ts:49-53) has four members and 'incoherent-mixed' is not one of them — it's the only occurrence of that string in the package. commands/db/status.ts:110 passes ore.state straight into describeOreState(state: OreSurfaceState):

src/commands/db/status.ts(110,44): error TS2345:
  Type '"incoherent-mixed"' is not assignable to type 'OreSurfaceState'.

Reproduced with the package's own tsc. Worth saying plainly: packages/cli already has ~15 pre-existing tsc errors (missing columnExists export, unbuilt @cipherstash/stack-drizzle, auth@0.42 wasm-types drift), so this isn't a break from green, and CI wouldn't catch it either way — there's no typecheck script here and tsup/esbuild strips types without checking. But it's an invented union member, and describeOreState's switch has no arm for a value the type says can arrive.

Smaller — happy for these to be follow-ups

installer/index.ts:203-212. The lock-budget error is a plain Error, so it misses the TlsVerificationError | EqlReinstallConnectionError | EqlReinstallRefusalError passthrough and gets wrapped into a message carrying two separate "nothing happened" clauses, reporting lock contention as an install failure. Give acquireLifecycleLock its own error class.

installer/verify.ts:665. In the manageTransaction: false path the success branch does ROLLBACK TO SAVEPOINT installed_eql_surface_read and RELEASE SAVEPOINT before the functionSignatures loop and the object literal that follow. Any throw after that point lands in the catch, which unconditionally re-issues ROLLBACK TO SAVEPOINT against a savepoint that no longer exists — Postgres answers 25P01, and that replaces the original error the caller needed. Move the release to the end, or track whether it already ran.

derived-search-index-restoration.ts:571 (rebuildIndexes). Two pieces of index metadata are dropped silently and aren't covered by the verification block (which compares definition / valid / ready / clustered / replident / comment):

• ownership — the rebuilt index is owned by the connecting role, so an index previously owned by another role changes owner across an eql upgrade;
• per-column statistics targets (ALTER INDEX … ALTER COLUMN n SET STATISTICS), which pg_get_indexdef() does not emit.

Both are recoverable by capturing pg_class.relowner / pg_attribute.attstattarget alongside the definition. At minimum they belong in the changeset's limits list, which currently reads as though the index comes back unchanged.

installation-state.ts:78-79. CAPABILITIES_SQL still derives eql_v3_present / eql_v3_internal_present from information_schema.schemata, while the presence query 30 lines above now uses to_regnamespace. information_schema.schemata only lists schemas the current role owns or holds a privilege on, so a single assessEqlInstallation({ includeCapabilities: true }) result can report v3.status === 'installed' while capabilities.preflight.eqlV3SchemaPresent === false for the same schema — e.g. a Supabase role with no grant on a postgres-owned eql_v3. Only commands/db/preflight.ts:162 renders it today, so the damage is a wrong "absent" line, but two probes in one snapshot shouldn't disagree.

installer/__tests__/installation-state.test.ts:78,135. Both tests hardcode the mock's answer to eql_v3.version() as '3.0.5', pinning them to the current bundle version. On the next EQL bump readOreStateAgainst short-circuits to not-comparable before reaching the branch under test, and the ORE-unavailable case fails with an unrelated message. Read the expected version from releaseManifest.eqlVersion.

Notes, no action needed

• verifyEqlSurface, EQLInstaller.isInstalled and EQLInstaller.getInstalledVersion now have no production callers, but are still exported and still carry their old comments.
• getInstalledVersion's behaviour is unchanged by the refactor (a version() that throws or returns falsy still yields 'unknown'), and isInstalled's move to to_regnamespace is a strict improvement over information_schema.schemata.
• I checked the search_path = '' question the rewritten comment glosses over: moving the eql_v3.version() probe inside the pinned-search_path transaction is safe, because pin_search_path_v3.sql gives every eql_v3 function a SET search_path clause and version()'s body is a constant SELECT anyway.

🤖 Reviewed with Claude Code

[cipherstash-bot]

Coverage review: the new safe-reinstall tests cover the happy path, unsafe dependency refusal, lock waiting, and reconstruction failure. I found two remaining branch gaps worth adding around refusal on malformed captured index metadata and rollback when post-rebuild verification rejects the rebuilt catalog shape.

[freshtonic]

Approving at 3867928. Every item from my previous review is addressed — I verified the blocker resolution first-hand, and the rest against the head diff and CI logs.

The blocker is properly dead. turbo.json's test task now declares env: ["STASH_TEST_DATABASE_URL"], and the head CI logs show the live suites executing on both matrix legs — reinstall.live.test.ts (20 tests) ✓ on Node 22/PG16 and Node 24/PG17, plus the released-baseline upgrade test, and the revival of the previously-dead verify.live (5), applied.live (10), preflight.live (3) and guarded-grants.live (2) suites. The new meta-test (scripts/__tests__/cli-live-postgres-ci.test.mjs) pinning the turbo env, workflow env, matrix and singleFork contract is exactly the right move — this failure mode is now regression-proof rather than merely fixed. The PG16/PG17 matrix split exercising both attstattarget representations is a thoughtful touch.

The cross-version concern got a better fix than I asked for. Instead of patching the bundle-derived exemption, the classifier now reads the installed catalog — an operator or cast implemented by an eql_v3/eql_v3_internal function is EQL-owned regardless of what the incoming bundle declares — which kills the healthy-database-refused-on-upgrade case, the lower() seq-scan hazard, and the bind-parameter plumbing in one stroke, with live tests covering the legacy-cast and customer-duplicate-signature directions plus a real released-baseline → head upgrade round-tripping genuine ciphertext. One narrow trade to be aware of (fine by me, worth a sentence in the changeset if you're in there): a customer-created public operator implemented by an EQL-schema function is now classified EQL-owned and dropped without refusal — defensible, since such an object cannot survive the schema drop anyway, and a customer operator in their own schema still refuses.

Everything else from the list: scaffolding gone (now #962, as asked); body's review guide and the jit description corrected; the five-minute lock comment fixed; the protect-ffi import is type-only with a dynamic beforeAll import. The delta's new scope (replica identity, index comments, per-column statistics targets, owner verification, typed lock-timeout and refusal errors with clean CLI rendering) stays injection-clean — all server-side format('%I'/'%L') — fail-closed on malformed catalog metadata, inside the same single transaction under the advisory lock, with rollback tests for each verification failure. Changeset correctly re-leveled to minor and accurate; skills/stash-cli in step.

Non-blocking nits for a follow-up or pre-merge tidy: the body's "Commit structure" section is stale again (names two commits that don't exist on an 11-commit branch — if it describes intended squash structure, say so); EqlLifecycleLockTimeoutError isn't special-cased in install.ts/upgrade.ts so a lock timeout renders via the crash path (message is actionable, presentation only); packages/cli/README.md:484 still types getInstalledVersion() as Promise<string | null> though it now returns 'unknown' for installed-but-unversioned (the skill documents the tristate correctly — per our convention, README fixes can ride a follow-up); and AGENTS.md's "one exemption today" count for the EQL-pins linter is now off by one.

This is a substantially stronger PR than the one I first reviewed. Nice work.