Simple-mode Form-B insert can misparse against a stale schema cache after fast DDL #39

Closed
opened 2026-06-22 14:03:34 +01:00 by claude-clouddev1 · 1 comment
Collaborator

Summary

A simple-mode Form B insert (insert into <T> values (…), no explicit
column list) submitted immediately after a DDL command that changed the
table can be wrongly rejected as "looks like SQL" (the
project.also_valid_sql / usage-error path), even though the exact same
command succeeds when entered at human speed.

Interactive users are not affected — the gap closes in milliseconds. It only
bites faster-than-human input: pasting several commands at once, scripted
input, or tests driving the TUI.

Reproduction (PTY, back-to-back, no pause)

create table Customers with pk id(serial)
add column to Customers: Name (text)
insert into Customers values ('Alice')      <- rejected: "trying to write SQL?"

Entered with a pause after add column (so the schema settles), the identical
insert into Customers values ('Alice') succeeds. Verified via a pseudo-terminal
harness: back-to-back fails deterministically; paced passes.

Diagnosis

Form B has no column list, so the walker derives the target columns from the
schema cache (SchemaCache, src/completion.rs:53, held as
App::schema_cache; Form B handling at src/dsl/walker/context.rs:155-157).
Parsing/validation runs synchronously in update() against that cache.

But the cache is refreshed asynchronously after the worker applies a DDL
command
(e.g. add column, which goes through the rebuild-table primitive).
In the window between submitting the DDL and the cache refresh landing, a
quickly-submitted Form B insert sees the pre-DDL schema (here: Customers
with only the auto id), so its value arity doesn't match → the simple-mode
insert fails to validate → the friendly layer adds the "also valid as SQL?"
pointer.

So: identical input, different result depending on whether the schema-cache
refresh has caught up. A value-bearing Form A insert (insert into T (Name) values ('Alice')) is less exposed because it names its columns explicitly.

Possible fixes (for whoever picks this up)

  • Make the schema-cache refresh after a mutation complete before the next
    input command is validated (sequence the refresh with command execution), or
  • Validate Form B arity against the authoritative schema (the worker/db)
    rather than the cached copy, or
  • Briefly gate input submission while a schema-changing command is in flight.

Impact / priority

Low — no interactive-user impact; the demo cast driver already paces, and the
new Tier-4 PTY tests (TT4) pace each command to completion (the correct way to
drive a TUI), so they are unaffected. Filed so the latent behavior isn't lost.

Discovered during TT4 (Tier-4 PTY end-to-end) implementation, 2026-06-22.

## Summary A simple-mode **Form B** insert (`insert into <T> values (…)`, no explicit column list) submitted **immediately after a DDL command** that changed the table can be wrongly rejected as *"looks like SQL"* (the `project.also_valid_sql` / usage-error path), even though the exact same command succeeds when entered at human speed. Interactive users are not affected — the gap closes in milliseconds. It only bites **faster-than-human input**: pasting several commands at once, scripted input, or tests driving the TUI. ## Reproduction (PTY, back-to-back, no pause) ``` create table Customers with pk id(serial) add column to Customers: Name (text) insert into Customers values ('Alice') <- rejected: "trying to write SQL?" ``` Entered with a pause after `add column` (so the schema settles), the identical `insert into Customers values ('Alice')` succeeds. Verified via a pseudo-terminal harness: back-to-back fails deterministically; paced passes. ## Diagnosis Form B has no column list, so the walker derives the target columns from the **schema cache** (`SchemaCache`, `src/completion.rs:53`, held as `App::schema_cache`; Form B handling at `src/dsl/walker/context.rs:155-157`). Parsing/validation runs synchronously in `update()` against that cache. But the cache is **refreshed asynchronously after the worker applies a DDL command** (e.g. `add column`, which goes through the rebuild-table primitive). In the window between submitting the DDL and the cache refresh landing, a quickly-submitted Form B insert sees the *pre-DDL* schema (here: `Customers` with only the auto `id`), so its value arity doesn't match → the simple-mode insert fails to validate → the friendly layer adds the "also valid as SQL?" pointer. So: identical input, different result depending on whether the schema-cache refresh has caught up. A value-bearing Form A insert (`insert into T (Name) values ('Alice')`) is less exposed because it names its columns explicitly. ## Possible fixes (for whoever picks this up) - Make the schema-cache refresh after a mutation **complete before** the next input command is validated (sequence the refresh with command execution), or - Validate Form B arity against the **authoritative** schema (the worker/db) rather than the cached copy, or - Briefly gate input submission while a schema-changing command is in flight. ## Impact / priority Low — no interactive-user impact; the demo cast driver already paces, and the new Tier-4 PTY tests (TT4) pace each command to completion (the correct way to drive a TUI), so they are unaffected. Filed so the latent behavior isn't lost. Discovered during TT4 (Tier-4 PTY end-to-end) implementation, 2026-06-22.
claude-clouddev1 added the bug label 2026-06-22 14:03:34 +01:00
Author
Collaborator

Fixed in 07575da (on main, not yet pushed).

Root cause confirmed as diagnosed: App::update validates submissions against App::schema_cache, which the runtime refreshes asynchronously after the worker applies a command (a SchemaCacheRefreshed event posted on the same FIFO channel as key events). Under faster-than-human input the follow-up Enter is processed before the refresh lands, so a Form-B insert sees the pre-DDL schema and is rejected as "trying to write SQL?".

Fix — submission gate (src/app.rs). dispatch_dsl arms awaiting_schema_refresh on every ExecuteDsl dispatch; while armed, further DSL submissions are held in a held_submissions FIFO instead of validated; the SchemaCacheRefreshed handler drains them against the now-fresh cache in submission order. App-lifecycle commands (quit/help/load/undo/rebuild) bypass the gate. Arm-on-every-dispatch keeps exactly one DSL command in flight, so the boolean gate is provably correct (refreshes are strictly one-per-dispatch and in order).

Scope: interactive input only — the replay/history-log/startup rebuild path already re-snapshots the schema synchronously inline before each line (run_replay), so it always had this guarantee; the fix brings the interactive path in line with it. No interactive-user impact (the gate clears in milliseconds); held input is never lost (a refresh always follows a dispatch, success or failure).

Tests (both verified red→green):

  • app::tests::form_b_insert_after_ddl_is_held_until_refresh_then_dispatched — Tier-1, deterministic event ordering.
  • e2e_pty::back_to_back_insert_after_ddl_still_succeeds — Tier-4 PTY, the unpaced inverse of flow 3 (confirmed it times out without the fix).

Full suite green (2521 passed / 0 failed / 1 ignored), clippy -D warnings + fmt --check clean. Design recorded as ADR-0022 Amendment 8.

Fixed in `07575da` (on `main`, not yet pushed). **Root cause** confirmed as diagnosed: `App::update` validates submissions against `App::schema_cache`, which the runtime refreshes *asynchronously* after the worker applies a command (a `SchemaCacheRefreshed` event posted on the same FIFO channel as key events). Under faster-than-human input the follow-up Enter is processed before the refresh lands, so a Form-B insert sees the pre-DDL schema and is rejected as "trying to write SQL?". **Fix — submission gate (`src/app.rs`).** `dispatch_dsl` arms `awaiting_schema_refresh` on every `ExecuteDsl` dispatch; while armed, further DSL submissions are held in a `held_submissions` FIFO instead of validated; the `SchemaCacheRefreshed` handler drains them against the now-fresh cache in submission order. App-lifecycle commands (`quit`/`help`/`load`/`undo`/`rebuild`) bypass the gate. Arm-on-every-dispatch keeps exactly one DSL command in flight, so the boolean gate is provably correct (refreshes are strictly one-per-dispatch and in order). **Scope:** interactive input only — the `replay`/history-log/startup rebuild path already re-snapshots the schema synchronously inline before each line (`run_replay`), so it always had this guarantee; the fix brings the interactive path in line with it. No interactive-user impact (the gate clears in milliseconds); held input is never lost (a refresh always follows a dispatch, success or failure). **Tests** (both verified red→green): - `app::tests::form_b_insert_after_ddl_is_held_until_refresh_then_dispatched` — Tier-1, deterministic event ordering. - `e2e_pty::back_to_back_insert_after_ddl_still_succeeds` — Tier-4 PTY, the unpaced inverse of flow 3 (confirmed it times out without the fix). Full suite green (2521 passed / 0 failed / 1 ignored), `clippy -D warnings` + `fmt --check` clean. Design recorded as **ADR-0022 Amendment 8**.
Sign in to join this conversation.