diff --git a/docs/adr/0022-ambient-typing-assistance.md b/docs/adr/0022-ambient-typing-assistance.md index 1150d34..b6a8be8 100644 --- a/docs/adr/0022-ambient-typing-assistance.md +++ b/docs/adr/0022-ambient-typing-assistance.md @@ -824,6 +824,63 @@ of issue #4; no `AmbientHint` / renderer change. Covered by seed_count_hint_does_not_leak_once_the_count_or_a_clause_is_given, seed_count_hint_also_fires_after_a_column_fill_target}`. +## Amendment 8 — submission gate: hold input until the schema-cache refresh lands (2026-06-22) + +§9 established the schema cache as the source of truth for the +walker's schema-aware dispatch, "refreshed by the runtime … after +successful DDL". That refresh is **asynchronous**: the runtime applies +a command on the worker thread (ADR-0010) and only afterwards posts a +`SchemaCacheRefreshed` event back through the **same FIFO channel that +carries key events**. Between dispatching a DDL command and that event +landing, `schema_cache` is stale w.r.t. the command just run. + +Validation in `App::update` is pure-sync (a core invariant — it cannot +do a DB round-trip), so it can only consult that cache. Under +**faster-than-human input** (paste, scripted input, an unpaced PTY +driver), the next submission's Enter is already queued *ahead* of the +refresh event, so it is validated against the stale cache. A simple-mode +**Form-B insert** (`insert into T values (…)`, columns derived from the +cache) submitted right after `add column` then sees the pre-DDL columns, +its value arity can't match, and the friendly layer tags it *"trying to +write SQL?"* — even though the identical line succeeds at human speed +(issue **#39**). + +**Decision — gate submission on the pending refresh.** `App` carries +`awaiting_schema_refresh: bool` + a `held_submissions` FIFO queue. +`dispatch_dsl` arms the flag on **every** `ExecuteDsl` dispatch; while +armed, a new DSL submission is **held** (queued in submission order) +rather than validated. The `SchemaCacheRefreshed` handler clears the +flag and **drains** the queue against the now-fresh cache, stopping as +soon as a drained command re-arms the gate (so the remainder wait for +*its* refresh — order preserved). A held command that doesn't dispatch +(parse error / pre-flight rejection) leaves the gate open and the loop +continues. App-lifecycle commands (`quit` / `help` / `load` / `undo` / +`rebuild`) route through `dispatch_app_command` *before* `dispatch_dsl` +and so are never held. + +**Why arm on every dispatch, not just DDL.** It keeps at most one DSL +command in flight, so refreshes are strictly one-per-dispatch and in +order — making the gate a provably-correct boolean. Arming only on +schema-mutating commands would let a *preceding* non-DDL command's +refresh clear the gate early and drain a held Form-B insert against a +cache that predates the DDL — reintroducing the bug in a corner case. +(App-lifecycle commands also refresh but bypass the gate; they are +modal/picker-gated and so cannot overlap a rapid DSL paste, the only +thing this guards.) + +**Scope.** Interactive input only. The `replay` / history-log / startup +rebuild-from-text batch path already re-snapshots the schema +**synchronously, inline, before every line** (`run_replay`, +`build_schema_cache`), so it has always had this ordering guarantee; +this amendment brings the interactive path in line with it. No +interactive-user impact (the gate clears in milliseconds); held input is +never lost because the runtime sends a `SchemaCacheRefreshed` after +*every* dispatch, success or failure. Covered by +`app::tests::form_b_insert_after_ddl_is_held_until_refresh_then_dispatched` +(Tier-1, deterministic event ordering) and the Tier-4 PTY regression +`e2e_pty::back_to_back_insert_after_ddl_still_succeeds` (the unpaced +inverse of flow 3). + ## Out of scope Deliberately deferred to keep this ADR shippable as a single diff --git a/docs/adr/README.md b/docs/adr/README.md index 686156b..ff51058 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -34,7 +34,7 @@ This directory contains the project's ADRs, recorded per - [ADR-0019 — Friendly error layer (H1) and i18n message catalog](0019-friendly-error-layer-and-i18n.md) - [ADR-0020 — Tokenization layer for the DSL parser](0020-tokenization-layer-for-the-dsl-parser.md) — **Superseded by ADR-0024 (never implemented).** Specified a `chumsky`-over-tokens architecture (separate lexer, `define_keywords!`, `&[Token]` grammar). ADR-0024 adopted a scannerless hand-rolled walker and removed `chumsky` entirely; the lexer/keyword/token model here does not exist. Kept as institutional memory of the path not taken. - [ADR-0021 — Parser-as-source-of-truth for H1a (per-command usage in parse errors)](0021-parser-as-source-of-truth-for-h1a.md) — **Mechanism superseded by ADR-0024; H1a scope continued in ADR-0042.** The *intent* (show the command's grammar at the point of error) shipped — `usage_ids` on each `CommandNode`, the `parse.usage.*` templates, and the `available_commands` fallback all exist — but via grammar nodes, not the `chumsky` `UsageEntry` registry / `parse.token.*` keys described here (which were never built). -- [ADR-0022 — Ambient typing assistance: colour, hint panel, completion (I3 + I4)](0022-ambient-typing-assistance.md) — **Amendment 1 supersedes §12's simple-mode-only carve-out**: the unified mode-aware walker (ADR-0030/0031/0032) now speaks SQL, so advanced-mode ambient assistance is re-enabled. `ambient_hint_in_mode` + `hint_resolution_at_input_in_mode` + `expected_for_hint_snapshot` thread `Mode`; `render_hint_panel` calls ambient for all modes (no more advanced-mode `None`); the one-shot `:` sigil is stripped before the ambient walk. Fixes a live bug where advanced-mode SQL hinting/completion-preview were dead despite Phase 2 marking them green (validated at the engine layer, not the UI). Simple-mode gating, highlighting, and the §13 performance posture are unchanged; covered by an app-level render test plus ambient-layer regression locks; **Amendment 2 reverses the handoff-14 keywords-first candidate ordering** — schema identifiers (table/column/relationship names) now sort *before* keywords so a name the user would have to look up stays visible in the single-row, window-scrolled candidate line (keywords are learned over time; the `tok_identifier`/`tok_keyword` colour split marks the boundary); shipped with a `walk_repeated` fix that surfaces a list item's trailing optionals at a clean boundary (`order by Name ` → `asc`/`desc`, `select Name ` → `as`, `create table … Code(text) ` → `not`/`unique`/`default`/`check`; the `,` separator deliberately not surfaced); records a deferred two-line hint box for growing lists; **Amendment 3 makes the ambient-hint fallback rung schema-aware** — Amendment 1's bottom-rung `parse_command_in_mode` was schemaless while every earlier rung was not, so between-values insert hints pointed at `)` (type-blind close) instead of `,` and wrong-arity closed tuples read "submit with Enter" for an input the schema-aware parse rejects (issue #2); now uses `parse_command_with_schema_in_mode`, no extra walk, with the friendly arity diagnostic still winning at its higher rung; **Amendment 4 gives column types a dedicated highlight class** — both `Node::Ident.highlight_override` *and* the `Word.highlight_override` field were dead (driver destructured the former to `_`, `walk_word` hardcoded `Keyword`); now both wired through, with a new `HighlightClass::Type` + eighth `Theme` field `tok_type` (a pink/deep-magenta distinct from both keyword purple and identifier teal) so types no longer render identically to identifiers (issue #8); the three `IdentSource::Types` slots opt in via `Some(Type)` (advanced-mode single-word SQL aliases — `float`, `varchar`, … per ADR-0035 §3 — ride along for free), and the two-word `double precision` alias opts in via the new `Word::type_keyword` constructor so it matches its synonyms; **Amendment 5 lets the hint panel grow for long prose hints** — a fixed one-row panel clipped long field-value/usage hints past the first line (issue #12); `resolve_hint_lines` now pre-wraps prose and `render_right_column` sizes the panel to the line count (1 row default, up to `MAX_HINT_ROWS`=3, reclaimed when short) with a `clamp_wrapped` ellipsis backstop; the candidate list still scrolls horizontally on one row (Amendment 2's deferred two-line candidate box stays deferred); also shortens the 299-char `parse.usage.sql_create_table` synopsis to a terse one-liner (full grammar remains in `help.ddl.sql_create_table`); **Amendment 6 adds a curated SQL function-name list** (`src/dsl/sql_functions.rs`, `KNOWN_SQL_FUNCTIONS` — aggregates + common + broader scalars; `cast` deliberately excluded as its `CAST(x AS type)` syntax isn't a plain-call shape) as the single source of truth shared by two consumers at the `sql_expr_ident` slot (ADR-0031 §1): **issue #15** offers the functions as Tab candidates under a new `CandidateKind::Function` + ninth `Theme` colour `tok_function` (a blue distinct from keyword/identifier/type, parallel to Amendment 4's `tok_type`) so a learner discovers `sum`/`upper`/…; **issue #16** restores the typing-time column-typo flag the issue-#6 fix had dropped wholesale at this slot — `invalid_ident_at_cursor` now bails only when the partial prefix-matches a known function, else falls through to the schema-column check, so `select Agx` warns again at typing time while `select sum` does not (the issue-#6 lockdown tests + the submit-time `unknown_column` diagnostic path are untouched, and the no-validation-allowlist posture stands); see ADR-0031's status note for the grammar-side anchor; **Amendment 7 surfaces optional positional args in the hint panel** (issue #26): at `seed ▮` the optional row count (a bare `NumberLit` with no candidate) was invisible next to the `set`/`--seed` chips, and the resolver short-circuits on the already-complete command. Extends the issue-#4 `IntroProse` `HintMode` (ADR-0024 §HintMode-per-node) to survive trailing optionals: `walk_optional` stashes a skipped inner's `IntroProse` key into a new `WalkContext.surviving_intro_hint` (key + position) before the empty match clears `pending_hint_mode`, and the snapshot keeps it only when the skip position is the cursor (so it never leaks past a later-consumed `set …` clause or once the count is given); the resolver returns it ahead of the empty-expected short-circuit. The seed count is wrapped `Hinted{IntroProse("hint.seed_count")}`; prose names the count (default 20), the `.column` column-fill form, and `set`/`--seed` (user-chosen scope). Only `IntroProse` is carried; `ProseOnly`/`ForceProse` and the CREATE-TABLE element (a required `Repeated`) are untouched; no `AmbientHint`/renderer change +- [ADR-0022 — Ambient typing assistance: colour, hint panel, completion (I3 + I4)](0022-ambient-typing-assistance.md) — **Amendment 1 supersedes §12's simple-mode-only carve-out**: the unified mode-aware walker (ADR-0030/0031/0032) now speaks SQL, so advanced-mode ambient assistance is re-enabled. `ambient_hint_in_mode` + `hint_resolution_at_input_in_mode` + `expected_for_hint_snapshot` thread `Mode`; `render_hint_panel` calls ambient for all modes (no more advanced-mode `None`); the one-shot `:` sigil is stripped before the ambient walk. Fixes a live bug where advanced-mode SQL hinting/completion-preview were dead despite Phase 2 marking them green (validated at the engine layer, not the UI). Simple-mode gating, highlighting, and the §13 performance posture are unchanged; covered by an app-level render test plus ambient-layer regression locks; **Amendment 2 reverses the handoff-14 keywords-first candidate ordering** — schema identifiers (table/column/relationship names) now sort *before* keywords so a name the user would have to look up stays visible in the single-row, window-scrolled candidate line (keywords are learned over time; the `tok_identifier`/`tok_keyword` colour split marks the boundary); shipped with a `walk_repeated` fix that surfaces a list item's trailing optionals at a clean boundary (`order by Name ` → `asc`/`desc`, `select Name ` → `as`, `create table … Code(text) ` → `not`/`unique`/`default`/`check`; the `,` separator deliberately not surfaced); records a deferred two-line hint box for growing lists; **Amendment 3 makes the ambient-hint fallback rung schema-aware** — Amendment 1's bottom-rung `parse_command_in_mode` was schemaless while every earlier rung was not, so between-values insert hints pointed at `)` (type-blind close) instead of `,` and wrong-arity closed tuples read "submit with Enter" for an input the schema-aware parse rejects (issue #2); now uses `parse_command_with_schema_in_mode`, no extra walk, with the friendly arity diagnostic still winning at its higher rung; **Amendment 4 gives column types a dedicated highlight class** — both `Node::Ident.highlight_override` *and* the `Word.highlight_override` field were dead (driver destructured the former to `_`, `walk_word` hardcoded `Keyword`); now both wired through, with a new `HighlightClass::Type` + eighth `Theme` field `tok_type` (a pink/deep-magenta distinct from both keyword purple and identifier teal) so types no longer render identically to identifiers (issue #8); the three `IdentSource::Types` slots opt in via `Some(Type)` (advanced-mode single-word SQL aliases — `float`, `varchar`, … per ADR-0035 §3 — ride along for free), and the two-word `double precision` alias opts in via the new `Word::type_keyword` constructor so it matches its synonyms; **Amendment 5 lets the hint panel grow for long prose hints** — a fixed one-row panel clipped long field-value/usage hints past the first line (issue #12); `resolve_hint_lines` now pre-wraps prose and `render_right_column` sizes the panel to the line count (1 row default, up to `MAX_HINT_ROWS`=3, reclaimed when short) with a `clamp_wrapped` ellipsis backstop; the candidate list still scrolls horizontally on one row (Amendment 2's deferred two-line candidate box stays deferred); also shortens the 299-char `parse.usage.sql_create_table` synopsis to a terse one-liner (full grammar remains in `help.ddl.sql_create_table`); **Amendment 6 adds a curated SQL function-name list** (`src/dsl/sql_functions.rs`, `KNOWN_SQL_FUNCTIONS` — aggregates + common + broader scalars; `cast` deliberately excluded as its `CAST(x AS type)` syntax isn't a plain-call shape) as the single source of truth shared by two consumers at the `sql_expr_ident` slot (ADR-0031 §1): **issue #15** offers the functions as Tab candidates under a new `CandidateKind::Function` + ninth `Theme` colour `tok_function` (a blue distinct from keyword/identifier/type, parallel to Amendment 4's `tok_type`) so a learner discovers `sum`/`upper`/…; **issue #16** restores the typing-time column-typo flag the issue-#6 fix had dropped wholesale at this slot — `invalid_ident_at_cursor` now bails only when the partial prefix-matches a known function, else falls through to the schema-column check, so `select Agx` warns again at typing time while `select sum` does not (the issue-#6 lockdown tests + the submit-time `unknown_column` diagnostic path are untouched, and the no-validation-allowlist posture stands); see ADR-0031's status note for the grammar-side anchor; **Amendment 7 surfaces optional positional args in the hint panel** (issue #26): at `seed
▮` the optional row count (a bare `NumberLit` with no candidate) was invisible next to the `set`/`--seed` chips, and the resolver short-circuits on the already-complete command. Extends the issue-#4 `IntroProse` `HintMode` (ADR-0024 §HintMode-per-node) to survive trailing optionals: `walk_optional` stashes a skipped inner's `IntroProse` key into a new `WalkContext.surviving_intro_hint` (key + position) before the empty match clears `pending_hint_mode`, and the snapshot keeps it only when the skip position is the cursor (so it never leaks past a later-consumed `set …` clause or once the count is given); the resolver returns it ahead of the empty-expected short-circuit. The seed count is wrapped `Hinted{IntroProse("hint.seed_count")}`; prose names the count (default 20), the `.column` column-fill form, and `set`/`--seed` (user-chosen scope). Only `IntroProse` is carried; `ProseOnly`/`ForceProse` and the CREATE-TABLE element (a required `Repeated`) are untouched; no `AmbientHint`/renderer change; **Amendment 8 adds a submission gate so interactive input is held until the schema-cache refresh lands** (issue #39): §9's schema-cache refresh is asynchronous (the worker applies the command, then posts `SchemaCacheRefreshed` on the same FIFO channel as key events), so under faster-than-human input a follow-up submission was validated against the *stale* cache — a Form-B `insert into T values (…)` right after `add column` saw the pre-DDL columns and was wrongly rejected as "trying to write SQL?". `App` now arms `awaiting_schema_refresh` on every `ExecuteDsl` dispatch and holds further DSL submissions in a `held_submissions` FIFO, draining them against the fresh cache when the refresh arrives (order-preserving; app-lifecycle commands bypass the gate; arm-on-every-dispatch keeps exactly one DSL command in flight so the boolean is provably correct); interactive-only (the `replay`/batch path already re-snapshots synchronously per line), no interactive-user impact, held input never lost (a refresh always follows a dispatch); covered by a Tier-1 deterministic ordering test + a Tier-4 PTY back-to-back regression - [ADR-0023 — Unified declarative grammar tree](0023-unified-grammar-tree.md) — direction (superseded for execution detail by ADR-0024) - [ADR-0024 — Unified grammar tree: execution plan](0024-unified-grammar-tree-execution-plan.md) — **Accepted**, the executable spec — implemented (Phases A–F; Phase F shipped "minimal", `parser.rs` retained as the router — see the ADR's Phase F implementation note) - [ADR-0025 — Indexes](0025-indexes.md) — **Accepted** (**Amendment 1, 2026-05-25**: UNIQUE indexes admitted on the **advanced-mode** surface via `CREATE UNIQUE INDEX` — ADR-0035 §4d; the `IndexSchema.unique` flag round-trips through `project.yaml` with no new metadata table since the engine reports uniqueness natively; simple-mode `add unique index` stays deferred), `add index` / `drop index`, persistence, rebuild-table preservation, and items-list display (`C3` index portion + `S2`) diff --git a/src/app.rs b/src/app.rs index ad721b8..1cf33a7 100644 --- a/src/app.rs +++ b/src/app.rs @@ -377,6 +377,31 @@ pub struct App { /// by default; refreshed by the runtime on project load /// and after successful DDL. pub schema_cache: crate::completion::SchemaCache, + /// Issue #39: `true` while a dispatched DSL command's async + /// schema-cache refresh is still in flight. The runtime applies a + /// command on the worker thread and only *afterwards* sends back a + /// `SchemaCacheRefreshed` event; until that lands, `schema_cache` is + /// stale w.r.t. the command just dispatched. Validating a follow-up + /// submission against that stale cache is the issue #39 bug (a Form-B + /// insert after `add column` sees the pre-DDL columns and is wrongly + /// rejected as "trying to write SQL?"). While this flag is set, new + /// `dispatch_dsl` submissions are *held* in `held_submissions` rather + /// than validated, then drained when the refresh arrives. Armed on + /// every `ExecuteDsl` dispatch, so at most one *DSL* command is ever in + /// flight — refreshes are then strictly one-per-dispatch and in order, + /// keeping the gate a simple, provably-correct boolean. (App-lifecycle + /// commands — `load` / `undo` / `rebuild` — also refresh the cache but + /// bypass this gate; they are modal/picker-gated and so cannot overlap a + /// rapid DSL paste, the only thing this guards.) Cleared in the + /// `SchemaCacheRefreshed` handler. + awaiting_schema_refresh: bool, + /// Issue #39: submissions deferred while `awaiting_schema_refresh` is + /// set, in submission order. Each is the canonical input line plus the + /// effective mode it was submitted under; drained through + /// `dispatch_dsl` (re-validated against the now-fresh cache) when the + /// pending refresh lands. `push_history` already ran for these at + /// `submit` time, so draining re-enters at `dispatch_dsl`, not `submit`. + held_submissions: std::collections::VecDeque<(String, EffectiveMode)>, /// Whether the undo/snapshot machinery is active this session /// (ADR-0006 Amendment 1). `false` under the `--no-undo` CLI /// flag; the `undo` / `redo` commands then report undo is off @@ -596,6 +621,10 @@ impl App { modal: None, last_completion: None, schema_cache: crate::completion::SchemaCache::default(), + // Issue #39: no command is in flight at construction; the + // schema-refresh gate starts open with an empty hold queue. + awaiting_schema_refresh: false, + held_submissions: std::collections::VecDeque::new(), // Undo is on by default; the runtime flips this off for // a `--no-undo` session (ADR-0006 Amendment 1). undo_enabled: true, @@ -912,7 +941,24 @@ impl App { "schema cache refreshed", ); self.schema_cache = cache; - Vec::new() + // Issue #39: the in-flight command's refresh has landed, so + // the gate opens. Drain any submissions held while it was in + // flight, re-validating each against the now-fresh cache. + // Stop as soon as a drained command dispatches (re-arming the + // gate via its own `ExecuteDsl`): the remaining held commands + // then wait for *its* refresh, preserving submission order. A + // held command that does not dispatch (parse error / pre-flight + // rejection) leaves the gate open, so the loop continues to the + // next held submission. + self.awaiting_schema_refresh = false; + let mut actions = Vec::new(); + while !self.awaiting_schema_refresh { + let Some((input, submission_mode)) = self.held_submissions.pop_front() else { + break; + }; + actions.extend(self.dispatch_dsl(&input, submission_mode)); + } + actions } AppEvent::RelationshipsRefreshed(relationships) => { trace!(count = relationships.len(), "relationships refreshed"); @@ -1981,6 +2027,20 @@ impl App { } fn dispatch_dsl(&mut self, input: &str, submission_mode: EffectiveMode) -> Vec { + // Issue #39: if a previously-dispatched command's schema-cache + // refresh is still in flight, `schema_cache` is stale w.r.t. that + // command. Validating this submission now would race it (the bug: + // a Form-B insert after `add column` rejected against the pre-DDL + // schema). Hold it in submission order; the `SchemaCacheRefreshed` + // handler drains the queue once the fresh schema lands. App-level + // commands (`quit`, `help`, `load`, …) route through + // `dispatch_app_command` *before* here, so they are never held. + if self.awaiting_schema_refresh { + debug!(input, "holding submission until schema cache refresh lands"); + self.held_submissions + .push_back((input.to_string(), submission_mode)); + return Vec::new(); + } // The two-way mode the walker + the `[mode]` render tag read; the // three-way `submission_mode` (ADR-0037) rides on `ExecuteDsl` for // the runtime's echo gate (ADR-0038). @@ -2069,6 +2129,14 @@ impl App { }]; } self.push_output(OutputLine::echo(input, mode)); + // Issue #39: a command is now in flight; its schema-cache + // refresh will land asynchronously. Arm the gate so any + // follow-up submission waits for the fresh schema rather + // than racing the stale cache. Armed on every dispatch (not + // just DDL) so only one command is ever in flight, which + // keeps the gate a simple boolean — refreshes are then + // strictly one-per-dispatch and in order. + self.awaiting_schema_refresh = true; vec![Action::ExecuteDsl { command: cmd, source: input.to_string(), @@ -4614,6 +4682,100 @@ mod tests { ); } + #[test] + fn form_b_insert_after_ddl_is_held_until_refresh_then_dispatched() { + // Issue #39: a simple-mode Form-B insert (`insert into T values + // (…)`, no column list) submitted faster than the post-DDL + // schema-cache refresh must NOT be validated against the *stale* + // cache — doing so wrongly rejects it as "trying to write SQL?". + // + // This reproduces, deterministically, the event ordering the async + // runtime produces under fast input: a schema-mutating command is + // dispatched (arming the gate while its cache refresh is in flight), + // a follow-up insert is submitted before the refresh lands, and only + // then does the `SchemaCacheRefreshed` event arrive. Pre-fix the + // insert is validated against the pre-DDL schema and rejected; with + // the gate it is held and dispatched against the fresh schema. + use crate::completion::{SchemaCache, TableColumn}; + use crate::dsl::types::Type; + + // Pre-DDL schema: `Customers` has only the auto `id` (serial). This + // is the stale cache a racing Form-B insert would otherwise see — + // zero user-fillable columns, so `values ('Alice')` can't match. + let mut app = App::new(); + let mut stale = SchemaCache::default(); + stale.tables.push("Customers".to_string()); + stale.columns.push("id".to_string()); + stale.table_columns.insert( + "Customers".to_string(), + vec![TableColumn { + name: "id".to_string(), + user_type: Type::Serial, + not_null: true, + has_default: false, + }], + ); + app.schema_cache = stale; + + // 1. Submit the DDL. It dispatches and arms the gate: a schema + // refresh is now (conceptually) in flight. + type_str(&mut app, "add column to Customers: Name (text)"); + let ddl_actions = submit(&mut app); + assert!( + matches!(ddl_actions.as_slice(), [Action::ExecuteDsl { .. }]), + "the DDL should dispatch; got {ddl_actions:?}", + ); + + // 2. Submit the Form-B insert *before* the refresh lands. It must be + // held — not dispatched, and crucially not rejected against the + // stale cache (no error note such as "trying to write SQL?"). + type_str(&mut app, "insert into Customers values ('Alice')"); + let held_actions = submit(&mut app); + assert!( + held_actions.is_empty(), + "the insert must be held while the refresh is in flight, \ + not dispatched or rejected; got {held_actions:?}", + ); + assert!( + !app.output.iter().any(|l| l.kind == OutputKind::Error), + "a held insert must produce no error note (e.g. the \ + 'trying to write SQL?' pointer); errors:\n{}", + error_lines(&app), + ); + + // 3. The DDL's schema refresh lands carrying the fresh schema + // (`Customers` now has `id` + `Name`). The held insert drains and + // dispatches, validated against the up-to-date cache. + let mut fresh = SchemaCache::default(); + fresh.tables.push("Customers".to_string()); + fresh.columns.push("id".to_string()); + fresh.columns.push("Name".to_string()); + fresh.table_columns.insert( + "Customers".to_string(), + vec![ + TableColumn { + name: "id".to_string(), + user_type: Type::Serial, + not_null: true, + has_default: false, + }, + TableColumn::new("Name", Type::Text), + ], + ); + let drained = app.update(AppEvent::SchemaCacheRefreshed(fresh)); + assert!( + matches!( + drained.as_slice(), + [Action::ExecuteDsl { + command: Command::Insert { .. }, + .. + }] + ), + "the held insert must dispatch once the fresh schema lands; \ + got {drained:?}", + ); + } + #[test] fn simple_mode_submit_of_pure_dsl_error_has_no_advanced_pointer() { // A DSL error that is *not* valid SQL either (unknown command) diff --git a/tests/e2e_pty.rs b/tests/e2e_pty.rs index 1bf9a8e..7eadab6 100644 --- a/tests/e2e_pty.rs +++ b/tests/e2e_pty.rs @@ -377,6 +377,33 @@ fn undo_after_drop_table_restores_it() { app.quit(); } +/// Flow 5 — issue #39 regression: commands submitted **back-to-back**, with +/// no wait between them, must still execute correctly. This is the inverse of +/// flow 3 (which paces each command on purpose). Pre-fix, a Form-B insert sent +/// immediately after `add column` was validated against the *stale* schema +/// cache — the worker hadn't yet refreshed it — and wrongly rejected as +/// "trying to write SQL?", so the row never landed. The schema-refresh gate +/// (`App::awaiting_schema_refresh`) now holds each submission until the prior +/// command's refresh arrives, making the outcome independent of input speed. +#[test] +fn back_to_back_insert_after_ddl_still_succeeds() { + let mut app = PtyApp::launch(&[]); + app.wait_for_no_tables(); // fresh project + + // Fire all three with no readiness wait in between — the faster-than-human + // input that triggered issue #39 (paste / script / unpaced driver). + app.submit("create table Customers with pk id(serial)"); + app.submit("add column to Customers: Name (text)"); + app.submit("insert into Customers values ('Alice')"); + + // The insert's OWN success echo (value + ✓) — proof the row reached the + // database, not the "trying to write SQL?" rejection. If the gate + // regressed, the insert would misparse against the stale schema and this + // would time out. + app.wait_for("('Alice') ✓"); + app.quit(); +} + // ===================== NFR perf (measured, generous) =================== // // These run against the DEBUG binary, so the bounds are loose diff --git a/tests/it/walking_skeleton.rs b/tests/it/walking_skeleton.rs index c2dec15..928d1d9 100644 --- a/tests/it/walking_skeleton.rs +++ b/tests/it/walking_skeleton.rs @@ -170,6 +170,12 @@ fn colon_escape_in_simple_mode_is_one_shot() { echoed.text, ); + // Issue #39: dispatching `:select 1` arms the schema-refresh gate (a + // cache refresh is conceptually in flight). In the real runtime that + // refresh lands before the user types the next line; model it here so + // the follow-up submission is processed rather than held. + app.update(AppEvent::SchemaCacheRefreshed(app.schema_cache.clone())); + // Subsequent submission (unrecognised in simple mode) parse-errors, // not echoes — confirming the mode reverted. type_str(&mut app, "list things");