Unblock the SQL Editor e2e suites, stop dropping inserted SQL, and add a reverse FK drill - #393
Closed
huyplb wants to merge 10 commits into
Closed
Unblock the SQL Editor e2e suites, stop dropping inserted SQL, and add a reverse FK drill#393huyplb wants to merge 10 commits into
huyplb wants to merge 10 commits into
Conversation
The SQL editor laid every saved connection out as chips in a horizontal scroller. That is fine with four. This developer's app has 254, so almost all of them sat off-screen behind a scrollbar and there was no way to see what was selected without dragging through the row. It is a dropdown now. The trigger says what is selected — the name when it is one, a count when it is several — so the common question is answered without opening anything. Inside: a filter input and a checkbox per connection. The filter matches name, dialect, host, database and schema, not just the visible name. The row shows name and dialect while the tooltip carries host and database, and people are usually told to connect to a host or a database rather than to whatever the connection was nicknamed. Filtering is a view over the list, not an edit of it: a selected connection that scrolls out of the filter stays selected, and there is a test saying so — that is the mistake this shape invites. Not built on the shared Autocomplete: it is a single-value combobox (`value: string`, `onChange(value)`) and this is multi-select. Its theming and filter idiom are the reusable parts and the rest would have been fought. Eight tests, A/B'd against two deliberate breaks — filtering on name alone, and rendering the list while closed. Both fail the tests written for them. Verified in the running app against the real 254 connections: typing "oracle" narrows to 11, the checkbox toggles the store, and the trigger updates to the chosen name. Suite 3665 passed, eslint clean, typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Server Insights on Oracle answered:
ORA-00942: table or view "SYS"."V_$PARAMETER" does not exist
— Oracle: sessions / processes from v$parameter + v$session.
The error names a view the reader has never heard of, says it does not exist,
and the hint repeats what we tried. All three are unhelpful, and the first is
actively wrong: the view exists, and Oracle reports one you may not read as one
that is absent. Two separate things were wrong here.
**The message.** dbaPrivilegeRemedy reads the engine's error and answers with
the grant that fixes it. Which codes mean "ask for a privilege" is per-engine
knowledge — ORA-00942 on a V_$ view, 42501 on Postgres, 1227 on MySQL,
VIEW SERVER STATE on SQL Server, SQL0551N on Db2 — so it sits in @foxschema/sql
next to the probes rather than in the service that catches the exception. The
response also becomes 403 rather than 500: this is a refusal, not a fault.
It names the three views the panel reads, not SELECT_CATALOG_ROLE. Both work; a
DBA asked for the minimum can say yes faster. ORA-00942 on an ordinary table
still returns nothing, because that one really is missing and offering a grant
would send the reader after a privilege they already hold.
**The grants had never applied.** docker/init/oracle/02_seed.sql has granted
these since it was written, with a comment describing this exact confusion —
but both the init hook and the reseed run that file as `system`, and granting on
a SYS.V_$ view as `system` is ORA-01031. sqlplus continues past the error, so
the file looked correct and no demo user ever had the grants.
They move to 03_vviews_sysdba.sql, run as SYSDBA by both paths, inside the PDB
where the users live.
Verified against Oracle 23, whole cycle: revoke the grants and the pool probe
gives the reported ORA-00942; reseed and it returns 322 / 81.
Seven tests for the remedy, including the two cases worth getting wrong — a
genuinely missing table, and a non-permission failure — where it stays quiet.
Suite 3672 passed, typecheck clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The privileges grid lists every object with its DML and DDL columns, and a row holding no privileges shows — in both. The action that opens the grant editor was labelled "Edit" on every row regardless, so on the screen a new user actually sees — every row empty — the only way to set a permission was behind a verb describing something that is not there. Reported as "I didn't see any setting permission", which is exactly right: it was not visible. The button now reads Grant with a plus when the row holds nothing, and Edit with a pencil when it holds something. Same editor either way; only the label and icon change, so nothing moves and no testid does either. Revoke already got this right — it disables itself when there is nothing to revoke. The complement was missing. The help line above the sections said "Edit opens Grant / Revoke", which described the mechanism rather than telling anyone what to do. It now names Grant as the action and says what a dash in a row means. Five tests, A/B'd by putting the unconditional "Edit" back: the row-label test fails, the rest hold. One of them pins the emitter sentence, which is the part of that paragraph worth not losing while rewording around it. Suite 3677 passed, typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…its height back **Data Peek lost its foreign keys one level down.** Drilling through a key opens the next panel with a qualified name — `demo_a.customers` — while the schema cache holds tables bare, as `customers`. The resolver gave up on any name containing a dot, so the second peek in a chain found no table, produced no links, and every foreign key in it rendered as plain text. The `return undefined` was guarding something real: matching a bare cache entry for `inventory.products` while connected to `public` would hand row-edit the wrong table's primary key. So the qualifier is now compared rather than dropped, and only when the connection actually records a schema — on engines that do not, demanding a match would reject every qualified name and lose the keys for everyone, which is worse than a risk that needs two schemas holding the same table name to arise at all. The first test I wrote for this passed against the broken code. It asserted that the FK cell was inside a button, which it is not either way. The hint text below the grid renders only when `linkColumns` is non-empty, so it reads the thing that actually changed; with the old `return undefined` restored, it fails. **The toolbar took 132px — 18% of a 720px screen.** Two causes, one of them mine. The connection chips carried `flex-1`, so each grew to 319px soaking up free space and together pushed the rest of the row onto a second line; they now size between a floor and a ceiling. The row's own `min-h-11` and `py-1.5` were also more air than the controls need. Worth saying plainly: the dominant factor is not the toolbar. This profile has the appearance scale on `lg`, so `html` is 18px and every rem in the app is 12.5% larger than the design assumes. That is a preference, not a defect — switching it to `md` takes the same toolbar from 123px to 112px. Suite 3673 passed. Two failures under full parallel load — the code-cell worker sandbox and a CLI TUI screen — both pass in isolation and neither is in code this touches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last dialect e2e failure, on every engine at once. The test opens the
Snapshots pane after migrating and then returns to Compare — and Snapshots is
its own workspace since v3, so the toolbar's pane switcher, gated on
`activeView === 'sync'`, is not on that screen. Clicking the Compare pill
waited 15s for a control that had gone.
Two call sites: LokeeHistoryPage.openComparePane, and a raw clickWhen in
shared-flow that bypassed it. Both now select Sync first; the raw one goes
through the page object, which is where that knowledge belongs.
Worth recording how long this took to see. The error named
`sync-pane-history-btn`, so I spent several rounds proving that button was
fine — present, 54x47, visible, enabled, unobscured, and stable across eight
sampled frames. It was. The call log named a different locator further down,
`sync-pane-compare-btn`, and I had been reading a truncated error. The button I
was measuring was never the one failing.
Full matrix, all passing:
dialects/postgres 11
dialects/mysql, mariadb, mssql 33
dialects/oracle, db2, cockroachdb, sqlite 39 (5 skipped)
database-access-dialects, 7 engines 49
132 e2e tests, 0 failures — the first clean run since #385 landed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
schema-history was 6/6 before the v3 UI and 1/6 after — five failures, all
cascading from one.
`snapshotTarget()` clicked Snapshot target and then waited up to 30s for that
button to come back enabled. It never does: capture bumps the Lokee epoch, and
by the time things settle the toolbar's compare controls are gone from the DOM
— the button and the target picker both. The run reported a timeout for a
snapshot that had already succeeded, with the toast still on screen saying
"Snapshot v1 · 10 object change(s) · 10 objects".
So it waits for the outcome instead. The toast is the thing that means the
snapshot happened; the button coming back is incidental, and either ending is
accepted.
The last failure was the mirror image: a second snapshot is taken after looking
at the first, from the Snapshots workspace, where the button is not on screen
at all. `snapshotTarget()` now returns to the compare pane first, the same way
`openComparePane()` does.
schema-history 1/6 -> 6/6
dialects re-checked 22/22 (the page object is shared)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e Run button The v3 sidebar ships every section collapsed, and a collapsed section renders no content at all — so `sql-conn-check-*` and `sql-schema-explorer` were absent rather than merely scrolled away. Six suites had been failing on that since the v3 chrome landed. The sections are an exclusive accordion, so ticking a destination closes Schema; the tests that then read the tree reopen it, the way a reader would. `ensureSidebarSectionOpen` no longer throws when the screen has no sidebar (the Utilities workspace has none) — nothing to expand is not a failure. Two layout bugs surfaced with them, both mine. The destinations dropdown sat in a block wrapper, so `max-w` left it sized by its own content: it overflowed 93px onto the Run button and swallowed its clicks. `w-full` binds it to the wrapper. Widening the block then crushed the layout segmented control from 94px to 39px, clipping its second button behind `overflow-hidden` — visible and hit-testable, but not clickable. A segmented control has no slack to give, so it is `shrink-0`. sql-editor-sqlite: 5 failing to 0.
Data Peek could already follow a foreign key to its parent. This is the other direction — from a customer, the orders that point at it — which is the question asked before deleting or editing a row. Reuses what is here rather than adding a parallel path: `inboundForeignKeysFor` is the mirror of `foreignKeyLinksFor`, and `buildInboundDrilldown` mirrors `buildForeignKeyDrilldown`. The blueprint lib already finds inbound tables by name, which is enough to warn before a rename but not to build a query — a drill needs the FK's columns. The two sides are easy to swap and a swapped query still parses, so the FROM and the WHERE are pinned by tests and both were checked against a deliberately reversed implementation. A child whose parent columns are not all in the grid gets no button: a WHERE built from a partial key matches the wrong rows.
SqlEditorPane wires the insert handler on mount and clears it on unmount, and `insertAtCursor` returned void, so a call with no pane mounted went nowhere and no caller could tell. Clone Table and the table blueprint both live in the Utilities workspace, where the pane is unmounted: pressing Insert SQL set the status to "Clone SQL inserted into the editor", switched to the editor, and showed an empty tab. The text was gone and the app had said it arrived. The bridge now falls back to the active tab's buffer and returns whether anything took the text. The store supplies the writer — the bridge must not import the store, the dependency runs the other way. Found by the e2e suite once its own setup stopped failing first.
openTableBlueprint reached for the schema explorer wherever it was called from. Run on its own it passed, because setup left the app on the SQL Editor; run after the Clone Table tests it sat on the Utilities workspace, which has no sidebar at all, and spent 25s timing out on a tree that was never going to appear. Switching views first takes it to 1.1s.
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4021f6c1-a60e-4b20-aab1-f2afc63e3c35) |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this is a new branch
PR #391 was squash-merged into
mainas85cd9aed91, but it only carried 17 files — the branch was merged while it sat at40d097226b, and further commits were pushed to it afterwards. Those later commits never reachedmaineven though the PR reads as merged. This branch is cut fresh fromorigin/mainand carries the 10 unmerged commits, so nothing is stranded on a merged branch.Verified by content, not by patch-id:
App.tsx,ConnectionChips.tsxandsurfaces.guard.test.tsare already inmain; the destinations dropdown, the Oracle privilege remedy, and everything after are not.What is here
The e2e suites had been failing in setup since v3. The v3 sidebar ships every section collapsed, and a collapsed section renders no content at all — so
sql-conn-check-*andsql-schema-explorerwere absent, not merely scrolled out of view. Because six suites died inbeforeAll, every test inside was reported as skipped rather than failed, so nothing downstream had been exercised in a long time. The sections are an exclusive accordion, so ticking a destination closes Schema; tests that then read the tree reopen it, the way a reader would.A silent data-loss bug this uncovered.
insertAtCursorreturnedvoidand no-oped when no editor pane was mounted. Clone Table and the table blueprint both live in the Utilities workspace, where the pane is unmounted: pressing Insert SQL set the status to "Clone SQL inserted into the editor", switched views, and showed an empty tab. The app reported success while discarding the SQL. It now falls back to the active tab's buffer and returns whether anything took the text; the store supplies the writer, keeping the bridge→store dependency one-way.Two layout bugs, both introduced by the dropdown in this branch. The trigger sat in a
blockwrapper, somax-wleft it sized by its own content — it overflowed 93px onto the Run button and swallowed its clicks (measured: trigger398→522, Run438→524;elementFromPointat Run's centre returned the dropdown's label).w-fullbinds it to the wrapper. Widening the block then crushed the layout segmented control from 94px to 39px, clipping its second button behindoverflow-hidden— visible and hit-testable, but not clickable; a segmented control has no slack to give, so it isshrink-0.New feature — Data Peek "Referenced by". The reverse of the existing FK drill: from a customer, the orders that point at it — the question asked before deleting or editing a row. It reuses what is here rather than adding a parallel path (
inboundForeignKeysFormirrorsforeignKeyLinksFor;buildInboundDrilldownmirrorsbuildForeignKeyDrilldown). A child whose parent columns are not all in the grid gets no button, because a WHERE built from a partial key matches the wrong rows.Verification
tsc --noEmit -p apps/webcleanKnown, and deliberately not papered over
Apply clonestill fails (sql-editor-utilities). A pre-existing ~30s client-side stall in schema readiness eats the test's 120s budget. Measured:selectUtilityConnection46ms, click 33ms,waitReady30,086ms, select 61ms — and the server log shows every request finishing in single-digit ms with no clone-apply request ever sent. This branch touches neitherensureSchema,CloneTableModal, norschemaApi; it has simply been invisible while the suite could not get past its own setup. I did not raise the timeout to make it green — that would hide a 30-second wait a real user also sits through.🤖 Generated with Claude Code
Note
Medium Risk
Touches SQL editor state (insert fallback), Data Peek query building, and DBA probe error handling; e2e-only risk is low, but inbound FK and privilege messaging affect user-facing data navigation and admin workflows.
Overview
Unblocks SQL Editor and dialect e2e by aligning page objects with v3 sidebar behavior (collapsed accordion sections, Sync workspace before Compare/Snapshot) and fixes silent SQL loss when Insert runs from Utilities while the Monaco pane is unmounted.
SQL Editor UX: Destination “chips” become a filterable dropdown with a selection summary; toolbar connection chips use bounded width (
flex-initial,max-w-[17rem]) so Run and layout controls stay clickable. Insert SQL usesinsertAtCursor→ boolean plus a store fallback that appends to the active tab when no pane handler is mounted.Data Peek: Adds “Referenced by” inbound FK drill (
inboundForeignKeysFor,buildInboundDrilldown,drillDataPeekInbound) and fixes schema-qualifiedtableNamelookup so FK hints work on chained peeks. DB Access shows Grant (not Edit) on rows with no privileges yet.Server / Oracle: DBA utility failures map to actionable grant text via
dbaPrivilegeRemedy(403 when applicable). Oracle demo V_$ view grants run under SYSDBA in a separate init script so they actually apply.Reviewed by Cursor Bugbot for commit 4c8852e. Bugbot is set up for automated code reviews on this repo. Configure here.