v1.58.0 - A write that did not happen stops reporting success

Changed

• A migrate action somebody declined at the prompt reported success, and the prompt itself came from inside core. DDLExecutor called promptUser.confirm directly — the one thing ADR 0009 removed from DataExecutor, still in place here because the CI gate reads writes and that was an import — and a declined DROP TABLE came back as status: "success" with the cancellation mentioned only in warnings, so a caller reading status was told the table was gone. DDLExecutionOptions now carries the same confirm callback, src/commands/mutation-confirm.ts supplies the CLI's implementation on stderr like every other question dbcli asks, and DDLExecutionResult.status gained cancelled. A destructive operation with no handler and no --force is refused rather than defaulted either way, matching DataExecutor. With the import gone, scripts/check-core-no-stdout.ts now rejects an @/utils/prompts import anywhere under src/core/** — no module had to join the ratchet for that rule to hold, which is the evidence ddl-executor was the last one. ADR 0009 recorded this as an open gap one commit ago and now records it as closed, with the falsification condition naming the new rule (#70).

• MongoDB and Redis writes now ask before writing, and this breaks unattended scripts that never passed --force. DataExecutor performs the confirmation, and only SQL goes through it: insert, update and delete against MongoDB or Redis were issued straight from the command to the adapter, so dbcli delete orders --where '{}' emptied a collection without asking anybody, in a terminal or out of it — while the same command against PostgreSQL stopped and waited. All six branches now pass through confirmDirectMutation before an adapter is even built, with --force honoured in exactly that one place, so the three engines cannot answer the question differently. The prompt shows the statement the dry run would print and omits the parameter block, because a MongoDB or Redis statement carries its values inline. The consequence to plan for: dbcli's existing rule is that a non-interactive run cannot answer the prompt and therefore ends as status: "cancelled" having changed nothing, which is what SQL automation has always had to pass --force to avoid — MongoDB and Redis automation must now do the same, and a script that does not will stop writing rather than start failing silently. cancelled is consequently reachable on every engine, which is what the documentation already implied. tests/unit/commands/mongo-redis-confirmation.test.ts covers all six branches against a mock adapter, and tests/integration/mongo-redis-confirmation.test.ts re-asserts the important half against the real servers in docker-compose.test.yml — the document is still there, the key still holds its value — because "the adapter method was not called" is a statement about a mock, not about the data. That compose file gained a mongodb service on the default port, which the pre-existing mongo integration tests were already assuming and silently skipping without, and bun run test:docker now runs all of tests/integration rather than only tests/integration/adapters — it brought up six services and then exercised three of them (#70).

• status on the insert / update / delete result envelope gained cancelled and dry_run, and this is a breaking change to what those commands report. Declining at the confirmation prompt, previewing with --dry-run, and running a write that matched no rows were all one value: status: "success" with rows_affected: 0. The three are different events and a caller had no way to tell them apart. Worse, all three commands derived the audit entry's success flag from that field, so pressing N at the prompt wrote an audit record saying the write had happened — an audit log that lies is worse than none. A parallel field was rejected: it would have left status still saying success while outcome said cancelled, and every consumer reading only status — writeAuditEntry included — would have stayed wrong. Exit codes are unchanged: only error exits 1, because cancelling is a choice rather than a failure. The audit mapping now lives in one place (src/commands/mutation-audit.ts) instead of one copy per command; a dry run is recorded as successful because it did complete the preview it was asked for, a cancellation is recorded as unsuccessful, and metadata.outcome carries the exact ending in both cases, since a boolean cannot express "did not happen, did not fail". tests/unit/core/data-executor-outcome-status.test.ts pins all four endings as mutually distinguishable and asserts that neither cancellation nor dry run issues the statement. The guarantee is that no write is sent, not that the database is untouched: both paths still open a connection and read the table schema, because the SQL a dry run exists to show cannot be built without the column list, and a confirmation prompt has nothing to display until it is. tests/unit/commands/mutation-db-contact.test.ts states that boundary at the command level — execute never called, connect and getTableSchema called once each — so the wider claim cannot be assumed from the narrower test (#70).

• The confirmation prompt for insert / update / delete moved to stderr, so stdout is one JSON document whether or not the write was forced. The generated SQL, the destructive-delete warning, the parameter list, and the y/n question were all written to stdout ahead of the result envelope, which meant that every mutation nobody passed --force to produced stdout no parser could read — including with --format json, whose entire purpose is to be parsed. The bug survived because the JSON test forced the write and therefore skipped the confirmation altogether. All four now go to stderr, matching what audit clear already did for its own confirmation: a question addressed to a person is not part of the result. A terminal shows both streams, so nothing changes for the human being asked, and tests/unit/commands/mutation-output-characterisation.test.ts pins the block byte for byte on stderr while asserting JSON.parse succeeds on stdout for a non-forced run. promptUser.confirm writes to stderr for every caller now, not only these three, since a y/n question on the data channel is never what was wanted. Everything in the block is localised: Generated SQL: and Parameters: are new ceremony.* keys, and the destructive-delete warning and the question itself moved out of DataExecutor — MutationConfirmationRequest now carries destructive: boolean instead of a finished warning sentence and prompt string. The line is not "core cannot translate" — permission-guard now does, and blacklist-validator always did — it is that core states facts and the command layer chooses words: whether a delete can be undone is a fact, while the sentence shown about it, in what tone and on which stream, is presentation, and an embedder should be able to say it in its own product's voice. The refusal enforcePermissionForType throws is localised in place (errors.permission_requires_level), with the permission names interpolated verbatim since they are the values written in the config file. English output is unchanged in every case; tests/unit/i18n/ceremony-messages.test.ts checks key and placeholder parity between the two ceremony.json files, so an English string added without its translation now fails (#70).

• --recovery now covers failures the executor reports, not only failures it throws, and the recovery envelope replaces the result envelope rather than following it. A statement that failed inside DataExecutor returned a result with status: "error", which the command printed as the JSON result envelope before exiting 1 — --recovery was consulted only in the outer catch block, so the flag silently did nothing for the most common kind of write failure. Both failure paths now emit the recovery envelope, and a caller parsing stdout gets exactly one JSON document either way. Automation that parsed the result envelope out of a failed --recovery run must read the recovery envelope instead; automation that does not pass --recovery is unaffected (#70).

Added

• insert, update and delete say what happened in prose when a person is watching. These commands declared --format but never branched on it, so a human running one by hand got a JSON envelope and nothing else. In an interactive terminal they now print the affected row count and table, the elapsed time for work that actually ran, and — for a write that changed rows — the fact that dbcli has no automatic undo for it. Redirected or piped stdout is byte-for-byte the envelope it always was, and --format json keeps the envelope in a terminal too; --format text is deliberately not treated as a request for prose, since it is the flag's default and therefore appears on invocations that asked for nothing. Ceremony strings live in resources/lang/{en,zh-TW}/ceremony.json, following the precedent set by the shell strings rather than being merged into the general message file (#70).

• A write that fails in a terminal now says so in prose too, on stderr. Prose covered the endings a person cares least about — a successful write, a cancellation, a dry run — while the two that actually stop you got a raw JSON envelope on stdout: a blacklist refusal and a validation failure (a malformed --set, a missing --where). Both now go through printMutationFailure, which prints the reason as a sentence when somebody is watching and the same envelope, byte for byte, when nobody is. Human-mode failures — including the executor's own status: "error" — write to stderr, joining the PermissionError and ConnectionError branches that always did, so every human-facing failure is on the error stream while routine progress stays on stdout. A blacklist refusal gets a hint naming dbcli blacklist list; nothing else gets the --recovery line, because --recovery writes a plan for a statement that failed against the database and a refusal at this stage never reached one (#70).

• Core modules can no longer write to stdout, and CI fails if one starts. DataExecutor printed the generated SQL and blocked on promptUser.confirm from inside src/core, on the same stdout that carries the JSON envelope agents parse — and dist/core.mjs shipped a real import("@inquirer/prompts") so that a library consumer could be handed an interactive prompt it never asked for. Core now describes the pending mutation and asks the caller through a confirm callback; src/commands/mutation-confirm.ts holds the CLI's implementation. scripts/check-core-no-stdout.ts enforces the boundary in CI, with the 16 modules that predate the rule in a ratchet list that can only shrink. The reasoning, its alternatives, and the condition that would falsify it are recorded in docs/adr/0009-core-does-not-write-to-stdout.md (#70).

Fixed

• Deleting an Elasticsearch index needed data-admin, and three schema changes needed nothing but query-only. The classifier matched paths without looking at the method, so one function mis-tiered four different requests. DELETE /users removes an entire index and was classified DELETE — the delete-a-document tier — which meant data-admin could destroy an index while the SQL equivalent, DROP TABLE, has always required admin; DELETE /logs-* and DELETE /_all went the same way, at cluster scale. Worse, DELETE /users/_alias/a matched the _alias read rule and PUT /users/_mapping and PUT /users/_settings matched theirs, so a query-only credential could delete an alias and rewrite a mapping. A DELETE is now the DELETE tier only when it names a document (_doc/<id> or _source/<id>) and DROP — admin — otherwise, and a read rule requires a read method, with _search and _count also accepting POST because that is how a query with a body is sent. Anything whose scope cannot be established fails closed to admin: the cost of being wrong that way is a refusal a user can escalate, rather than an index nobody can get back. The adapter's own calls are unaffected — it reads mappings with GET and deletes documents through _doc/<id> — so this changes only what a raw request through query may do. tests/unit/core/elasticsearch-destructive-scope.test.ts pins all seventeen cases as a matrix of classification and tier (#70).

• The last four integration files that never ran anywhere now run everywhere. p1-error-classification, p2-explain and p3-missing-index were gated on TEST_MARIADB_HOST — an operator-supplied MariaDB that docker-compose.test.yml did not provide, so 23 assertions about MariaDB error codes, ANALYZE SELECT, and EXPLAIN output shape skipped on every machine and every CI run since they were written. The compose file gained a mariadb:11 service on port 3308 (its own service, not a MySQL alias: the codes and EXPLAIN shape are precisely what those tests distinguish), and the three files now gate on reachability like everything else. q-live was gated the same way on DBCLI_LIVE_PG_HOST, and wanted nothing more specific than a real PostgreSQL, which the stack already ships. Running it for the first time falsified one of its assertions: it expected SELECT :v AS x to return the number 1, and PostgreSQL types an untyped parameter as text — EXPLAIN VERBOSE shows '1'::text — so the value is the string "1". The assertion was wrong from the day it was written and no run had ever said so. tests/integration now reports 652 passing and 0 skipped (#70).

• q, query and the Elasticsearch path told users to grant a level that would not have helped. PermissionError.requiredPermission is what every command interpolates into Permission denied (required: …), and these three throw sites passed the level the caller already had — so a query-only user running DELETE through q read required: query-only above a sentence saying it needs data-admin. The structured-write path was corrected first (#72) by deriving the level at the throw site, which works only where the type alone decides it; a write hidden inside a read needs admin whatever its leading keyword says, a multi-statement SQL needs admin, and an unrecognised statement needs read-write. The level is now decided by the branch that decides the refusal and carried on PermissionCheckResult.requiredPermission, so the header and the reason cannot disagree — tests/unit/core/permission-refusal-level.test.ts asserts that granting exactly what a refusal names lets the same statement through, rather than pinning a table of strings. Elasticsearch stopped restating the tier table while it was being fixed: it was a third copy, agreeing with the shared one on every type its classifier produces, and its refusal now names the level instead of saying "requires higher permission tier", which is true of every refusal and tells nobody what to change (errors.elasticsearch_requires_level, both languages) (#70).

• The integration tests never ran in CI, and several of them never ran anywhere. The matrix job sets SKIP_INTEGRATION_TESTS=true, so tests/integration was compiled and skipped on every push — 639 assertions against MySQL, PostgreSQL, Redis, MongoDB and Elasticsearch that only ever executed on a developer's machine. A new integration job runs them on Linux against docker-compose.test.yml. The part that makes it a signal rather than a formality: REQUIRE_INTEGRATION_SERVICES=true turns the suite's auto-skip into a failure naming the address, because a job that starts no services otherwise reports exactly the same green as one that starts all of them, and bun run services:check fails first with a readable list, reading the ports out of the compose file rather than a copy that would drift. Turning that flag on immediately found the second half of the bug: verify-migration, verify-rollback, verify-safe-backfill and assert-verification-artifact dialled localhost:5432 as postgres/postgres — a server this repo has never shipped — while the adapter tests used PG_PORT at 5433 as dbcli/testpass, the one in the compose file. Two spellings for one address, and the files using the wrong one had been skipping since they were written, locally included. Connection defaults now live once in tests/integration/helpers.ts. (#70).

• insert / update / delete accepted any --format value and quietly did something else with it. The flag is declared as text or json, but nothing checked, and shouldRenderForHuman only special-cases json — so --format xml meant "prose in a terminal, envelope in a pipe", which is the default it was trying to override. An unsupported value is now refused before the connection, the schema read, and the audit write, the way dbcli export has always validated its own formats, with the offending value named and localised (errors.invalid_output_format). --plan is covered by the same guard (#70).

• Four refusals reached a zh-TW user as English, or as English glued to Chinese. The three sentences permission-guard builds for the cases the tier table cannot phrase — a write hidden inside a read, a multi-statement SQL below admin, and an unrecognised statement under query-only — were string literals, and handleMutationError prefixed the already-translated PermissionError message with a literal Permission denied: , producing a half-translated sentence. All four are catalogue keys now (errors.escalated_write_requires_admin, errors.multiple_statements_refused, errors.unknown_statement_query_only, errors.permission_denied_reason), with the permission levels and SQL keywords interpolated verbatim because those are values a user types into a config file. The English is character-for-character what it was — several tests assert these sentences and none of them changed — and tests/unit/i18n/permission-refusal-messages.test.ts renders each in both languages and checks key and placeholder parity across the two messages.json files, so an English string added without its translation fails (#70).

• An Elasticsearch insert / update / delete answered every user in Traditional Chinese. The "Elasticsearch does not support this command" sentence was a string literal in the error field of the envelope, in three files where everything else goes through t(). It is now {insert,update,delete}.elasticsearch_unsupported with English and zh-TW values, so the message follows DBCLI_LANG like the rest of the CLI. tests/unit/commands/redis-es-unsupported.test.ts asserts the English text under the default locale, which is what an English-locale user was never getting (#70).

• The three write paths' permission checks agreed by luck, and their refusal messages named levels that would not have worked. executeInsert and executeUpdate handed the classifier a synthetic statement ('INSERT INTO dummy') while executeDelete compared this.permission inline — one axis, two implementations, certain to drift. All three now call enforcePermissionForType, which skips the classifier because the caller assembled the statement and therefore already knows its type; passing the real generated SQL was tried and rejected, since it forces the statement to be built and its columns validated before the caller is known to be authorised, so an unauthorised user would learn Column not found first and the schema would leak. Separately, handleMutationError discarded the PermissionError and substituted a fixed sentence about query-only mode whatever the actual level was, and excluded delete outright. Refusals are now derived from the same tier table the decision uses, so they name the lowest level that actually permits the operation alongside the current one — previously a query-only user was told DELETE "requires read-write", which read-write does not grant. The header printed above that reason was wrong in the same way and is fixed with it: PermissionError.requiredPermission is what every command interpolates into Permission denied (required: …), and the SQL path passed the level the caller already had, so a query-only user read required: query-only directly above a sentence saying INSERT requires read-write. It now carries the level that would actually work, which is what the Redis enforcer has always passed and what delete used to hardcode. tests/unit/core/permission-refusal-level.test.ts asserts the named level really permits the operation rather than pinning a table of strings. q and the Elasticsearch enforcer still pass the current level; those refusals cover composite and hidden-write cases where "the level that would work" is not always a single answer, and they are left for a change that can decide it. The verdict matrix itself is unchanged: tests/unit/core/data-executor-permission-characterisation.test.ts pinned twelve verdicts before the unification landed, and only four messages moved. permission-guard.ts was 1187 lines by the end of this and is now 460: SQL lexical analysis, Redis, and Elasticsearch are three independent classification domains and moved to src/core/permission/{sql-analysis,redis,elasticsearch}.ts with no re-export shim — the six importers point at the new paths, and none of the moved symbols was on the published ./core surface. The three tier branches inside checkPermissionForClassification also stopped repeating what TIER_GRANTS already says: each tier's permitted set is derived by accumulating the tiers below it, which is the same drift-by-duplication this bullet removed from the refusal messages (#70).

---

Install: npm install -g @carllee1983/dbcli@1.58.0

Full diff: v1.57.0...v1.58.0