[tobyhede]

Requested by Toby

Schema middleware kept its own map of prepared statements and portals, recording whether each one runs DDL. That map sat outside Context's rollback journal and outside the statement lifecycle, so it could get out of step with PostgreSQL.

This PR is the first step of the operation-ledger refactor from the architecture review. It fixes the three bugs that review found before the larger restructuring starts.

Bugs

All three are reproduced by failing tests in the first commit (3d934b7) and fixed in the second.

1. Rejected re-Parse forgets that a statement is DDL.
   • Steps: prepare d as CREATE TABLE … and let PostgreSQL acknowledge it, then re-Parse d as SELECT 1. PostgreSQL rejects the re-Parse and keeps the DDL.
   • Context restored its own entry for d, but Schema middleware's map now said d was the SELECT.
   • Result: executing d ran DDL without the injected Flush, without updating the transaction schema overlay, and without schema publication.
2. Schema intents were never released.
   • Close, DEALLOCATE and DISCARD ALL left every intent in Schema middleware's map, each holding a full AST clone.
   • ProtocolStateSizes didn't count the map, so the bounded-state tests couldn't see it.
3. DISCARD ALL didn't end temporary-table shadowing.
   • A temporary table that shadows an encrypted table leaves the connection fail-closed.
   • DISCARD ALL drops that table, but the fail-closed state stayed set. DISCARD ALL is PgBouncer's default reset query, so the refusal carried over to every later client of that pooled connection.

Fix

• Schema middleware now only classifies a statement (SchemaMiddleware::intent) and applies outcomes. The intent is stored in Context's prepared statement entry, and bind_statement_effects copies it onto each portal bound from the statement, as it already does for a deallocation.
• MetadataSnapshot now records the intent, so the rollback journal restores it with the rest of the statement. Close and deallocation release it along with everything else in the entry.
• Schema middleware's prepared and portals maps are deleted. execute now takes the intent from Context instead of looking it up by portal name.
• DISCARD ALL clears the connection-local fail-closed state, because PostgreSQL refuses it inside a transaction block, so no rollback can restore the dropped tables. DISCARD TEMP still keeps the connection fail-closed, because a full rollback can bring the temporary table back.

This follows the Prepared statement definition in CONTEXT.md and ADR-0001's "prepared DDL records intent with the prepared statement". The CONTEXT.md entries for Prepared statement and Schema middleware are updated to match.

Tests

• New tests:
  • Re-Parse rejected by PostgreSQL keeps the DDL intent (red before the fix).
  • Re-Parse refused by Proxy refuses the retained statement. This already passed before the fix and pins the safe behaviour.
  • A baseline test that executing prepared DDL waits for its outcome.
  • Close of statements and portals retains nothing.
  • DISCARD ALL ends temporary shadowing, DISCARD TEMP keeps it, and DISCARD ALL keeps catalog-level unmodelled DDL fail-closed.
• In the first commit, the existing *_retains_nothing tests also count Schema middleware's maps, which made them fail. The fix deletes those maps, so the counters go too.
• cargo test -p cipherstash-proxy --lib passes (519 tests). cargo clippy --all-targets -D warnings and cargo fmt --check are clean. I couldn't run the integration tests locally because they need CipherStash credentials.

Acknowledgment

By submitting this pull request, I confirm that CipherStash can use, modify, copy, and redistribute this contribution, under the terms of CipherStash's choice.

---

Generated by Claude Code

[cipherstash-agent]

Automated review attestation

Source: Claude Code /code-review medium + company-skills:cross-check-review-findings, run against head 1a40c0e.

Result: no critical, blocking or new findings.

Checked against the head tree:

• Rollback of schema intent: statement_snapshot is taken before prepare_schema_statement (lifecycle.rs:99 vs :153), and restore_failed_preparation removes the entry before restoring schema. Both a rejected re-Parse and a Proxy-refused Parse (validate_setting failure) restore the prior intent.
• Portal copy: bind_statement_effects runs on both Ok arms of the Bind handler (frontend.rs:281, :286), so every bound portal carries its statement's intent. Error arms refuse the Bind and need no intent.
• Release: the intent lives in the prepared-statement and portal entries, so Close, DEALLOCATE and DISCARD ALL release it with the entry. The deleted prepared/portals maps have no remaining callers.
• DISCARD ALL: local_unmodelled is set only by temporary-table paths (middleware.rs:840, :971), so clearing it on a confirmed DISCARD ALL matches what PostgreSQL drops. PostgreSQL refuses it inside a transaction block, so no savepoint or rollback restore can resurrect the flag. The catalog-level unmodelled flag is untouched (pinned by discard_all_keeps_unmodelled_catalog_ddl_fail_closed).
• Existing feedback: none on the PR, so nothing to cross-reference.

Not run: the test suite (needs CipherStash credentials and a built workspace), so the claimed 519 passing lib tests are unverified here.

Nit, non-blocking: execute_schema_portal still clones the full SchemaIntent AST on every Execute, as the old portals lookup did. Passing Arc<SchemaIntent> through execute would avoid it in a later step of the refactor.

---

Generated by Claude Code