[ARCH.DB.1] Inventory and ratchet concrete persistence leakage - #203
[ARCH.DB.1] Inventory and ratchet concrete persistence leakage#203alseif0x wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3409947c6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please re-review the latest commit |
There was a problem hiding this comment.
💡 Codex Review
rustycore/tools/architecture/handler-contract-check/src/persistence_access.rs
Lines 916 to 920 in 41debe7
When code uses the legal grouped alias form use wow_database::{self as db};, this branch appends self to the source path and produces wow_database::self rather than treating db as an alias of the wow_database namespace. Consequently, both the import and a subsequent db::CharacterDatabase type reference are absent from the inventory, so this concrete database dependency can be introduced without tripping the non-growth snapshot. Handle renamed self leaves the same way the preceding UseTree::Name branch handles an unrenamed self leaf.
rustycore/tools/architecture/handler-contract-check/src/persistence_access.rs
Lines 2314 to 2315 in 41debe7
When a concrete pool is passed to an unrelated method whose name happens to be execute, fetch_one, or another executor name, operation is populated but the receiver validation above rejects the operation because the receiver has no persistence flow. This line nevertheless treats the call as known and suppresses argument-escape recording; for example, adding sink.execute(pool) where pool: &MySqlPool and sink is a local helper produces no new call or escape row beyond the already-baselined parameter. Only suppress escape handling when the persistence operation was actually validated and recorded.
rustycore/tools/architecture/handler-contract-check/src/persistence_access.rs
Lines 1086 to 1090 in 41debe7
When a tuple struct or tuple enum variant stores a concrete database value, symbol collection registers every unnamed field under the literal key tuple_field, while expression analysis looks up accesses such as .0 under the key 0. Thus, after a wrapper like struct Adapter(CharacterDatabase) is baselined, adding adapter.0.pool() does not produce a pool-access row because neither the wrapper parameter nor its numeric field carries persistence flow. Store unnamed fields using the same numeric member identity consumed by field_flow.
rustycore/tools/architecture/handler-contract-check/src/persistence_access.rs
Lines 1747 to 1748 in 41debe7
When a concrete persistence value flows through an expression variant not listed here, the analyzer silently drops the flow instead of rejecting or propagating it. For example, in a function whose pool: MySqlPool parameter is already baselined, let hidden = loop { break pool }; consume(hidden); adds no value-alias or argument-escape row: Expr::Loop takes this default, and visits to the known local paths are intentionally ignored. Unsupported arrays, loops, indexing, async blocks, and similar value-bearing expressions can therefore launder pools past the non-growth guard; conservatively propagate their child flow or fail closed when they contain a known persistence value.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| row.target == PersistenceTarget::SqlTransactionCommitError | ||
| || (row.operation == PersistenceOperation::Commit | ||
| && row.symbol == "commit_with_outcome_like_cpp") | ||
| || row.symbol.to_ascii_lowercase().contains("unknown") |
There was a problem hiding this comment.
Restrict unknown-outcome inference to commit semantics
When any persistence row's symbol merely contains unknown, this marks the entire workflow as requiring definite-rollback versus unknown-COMMIT reconciliation. The current generated policy therefore assigns that guarantee to Database::open_with_pool_size_and_auto_create_like_cpp solely because it calls is_unknown_database_error_like_cpp, even though that path only classifies a connection error and performs no commit or reconciliation. This leaves the semantic ledger inaccurate and can impose or preserve nonexistent failure behavior during later extractions; infer this guarantee only from outcome-aware commits or explicit reviewed reconciliation evidence.
Useful? React with 👍 / 👎.
Summary
Scope
This is an architecture-governance change only. It does not change runtime behavior, SQL statements or database schemas.
Validation
tools/pr-preflight.sh architecturetools/pr-preflight.sh quick 2d94ee51tools/pr-preflight.sh full 2d94ee51Closes #186