Skip to content

feat(schema): one blueprint everywhere, plus four editor/grid/API fixes - #259

Merged
huyplb merged 2 commits into
mainfrom
feat/shared-schema-blueprint
Aug 16, 2026
Merged

feat(schema): one blueprint everywhere, plus four editor/grid/API fixes#259
huyplb merged 2 commits into
mainfrom
feat/shared-schema-blueprint

Conversation

@huyplb

@huyplb huyplb commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Two themes: a reuse pass that makes version history render Compare Schema's own
components, and four defects found while reviewing the editor, the data grid and
the API surface.

One blueprint, three surfaces

History had grown its own thinner copy of every diff surface, because
ObjectDetailPanel and SchemaTreePanel read the live compare straight out of
useSyncStore. The copies drifted — history showed a flat varchar(100) → varchar(150) list where Compare Schema showed original/target cells with an
operation badge, and only the workspace copy ever gained the primary-key, index
and trigger sections. History never showed indexes at all, despite storing them.

Extracted, props-only, the way SchemaDiffTree already was:

component what it renders
SchemaBlueprint summary cards, routine params, sequence/type attrs, columns, PK, indexes, FKs, triggers
SchemaDdlDiff buildTableDdlDiffLines + the status-coloured CREATE TABLE
DetailTabs the canonical ids/labels — Schema Blueprint, DDL Diff, Migration SQL

Store wiring — deploy checkboxes, search highlight, the Monaco definition editor
— stays in the owning panel and is passed down as optional props or a slot;
omitting a handler hides its control rather than branching on a readOnly flag.

Compare Schema, the history compare modal and the graph inspector all render
them now. The inspector needed a TableDiff it did not have, so inspectObject
narrows both version states to the owner subtree and runs the same
CompareModule — one object's worth of compare, not the whole schema. Direction
matches diffVersions: source is the newer state, so ADDED reads as "this
version added it".

Also carried here: shape-level storage dedup (migration 14, measured 76% less
body storage on a synthetic 200×20 schema), per-object change kinds on the
graph, the container growth roadmap, and selective revert.

Four fixes

Run ignored the caret. With nothing checked and nothing selected, Run always
sent statement 1 — so an editor holding a page of queries silently re-ran the
top SELECT when the user pressed Run on their UPDATE further down. Run now
defaults to the statement under the caret; the tooltip names the cell
(In [3] at the caret) so the label cannot disagree with what executes.

Write confirmation could execute a different statement than it listed.
Resuming after the Safe-mode dialog re-derived the statements from the editor, so
a caret or checkbox change between Run and Confirm would swap them. The resolved
indices are pinned into the pending confirmation.

Wrong-row writes on case-colliding columns. Data-grid column lookup folds
case, so two result columns differing only by case — Postgres and Db2 both allow
"ID" beside "id" — collapse onto one index and the last wins. A key column
could read its value from the wrong column and the WHERE clause match a
different row. Editing is refused rather than guessed.

CSV formula injection (CWE-1236). Result grids export whatever the queried
database holds, and every major spreadsheet executes a cell starting with =,
+, -, @ or a tab. Both export paths — the .csv download and the
clipboard TSV, evaluated the same way on paste — prefix a single quote. Plain
numbers are exempt: quoting -5 would turn numeric columns into text and break
every SUM in the file.

POST /db/test had no restriction. Its comment said the route was limited
to the local edition "because a connection probe would be an SSRF vector";
nothing implemented that. The handler dials any host:port a caller names and
reports through the error text whether something answered, on a route with no
permission check. Now gated on isLocalSingleUser(), read per call so tests and
deployments both see the real value.

Review findings that are not code changes

  • No committed lockfile. package-lock.json is gitignored, so every
    workflow uses npm install rather than npm ci: builds are not reproducible
    and no integrity hashes are pinned. The overrides from fix(security): CodeQL Action v4 and npm audit overrides #252 do work — I
    confirmed by resolving the manifests in a clean directory (dompurify@3.4.13,
    adm-zip@0.6.0) — but they depend on every environment resolving fresh.
  • The audit gate only fails on critical (npm audit --audit-level=critical
    in audit:security and release-gate.yml), so the two current high-severity
    advisories pass CI.
  • Long-lived dev environments installed before fix(security): CodeQL Action v4 and npm audit overrides #252 still run the vulnerable
    versions
    — this machine had dompurify@3.2.7. Worth a reinstall.
  • Dialect coverage is complete. All 14 SQL dialects appear in every
    capability table (index fragmentation, DBA utilities, identity insert, FK and
    index features). AzureSQL/YugabyteDB/TiDB/MariaDB implement zero own hooks by
    design — they re-export SQL Server / Postgres / MySQL.

Verification

tsc --noEmit clean · 1594 tests pass, 31 skipped · eslint . 0 errors.

Verified live against a real Postgres history (demo_b, 3 versions, 109
objects): the compare modal renders Schema Blueprint / DDL Diff / Migration SQL
with Execute migration (6) in the toolbar; DDL Diff produces the coloured
CREATE TABLE legacy_audit_log with four + column lines; Migration SQL shows
Data loss · 5 lossy and 11 statements with no ;;.

Not verified: the revert has never actually been applied — Docker is down, so
there is no reachable database and the Execute path is proven only up to the
button. The caret fix is covered by unit tests but was not exercised in a live
browser; the preview's Monaco would not take synthetic input.

🤖 Generated with Claude Code


Note

High Risk
Touches security-sensitive /db/test gating, metadata schema migration, and revert/migration SQL generation where incorrect compare direction or selective revert could cause wrong DDL; broad UI refactor with heavy test coverage but large blast radius.

Overview
Unifies schema comparison UI by extracting SchemaBlueprint, SchemaDdlDiff, DetailTabs, and SchemaDiffTree so live Compare Schema, version-history compare, and the graph inspector render the same blueprint, DDL diff, and tab chrome instead of divergent copies.

Lokee weave gains diffVersions and GET …/compare (object-store-only diffs via CompareModule), inspectObject returns a scoped TableDiff, and revert plans/executes through migrationFromCompare with optional objectKeys (empty selection is a no-op). Storage adds migration 14 (lokee_shapes dedup), richer graph changeKinds, and container growth roadmaps over up to 500 versions.

Security: isLocalSingleUser() (read per call) returns 403 on POST /db/test when LOCAL_SINGLE_USER=false, closing an unauthenticated metadata-DB probe that behaved like SSRF/port scan.

Reviewed by Cursor Bugbot for commit f089081. Bugbot is set up for automated code reviews on this repo. Configure here.

huyplb and others added 2 commits August 16, 2026 01:42
History rendered its own, thinner version of every diff surface, because
ObjectDetailPanel and SchemaTreePanel read the live compare straight out of
useSyncStore. The copies drifted: history showed a flat "varchar(100) →
varchar(150)" list where Compare Schema showed original/target cells with an
operation badge, and only the workspace copy ever gained the primary-key,
index and trigger sections.

Extract the presentation, props-only, the way SchemaDiffTree already was:

  SchemaBlueprint  summary cards, routine params, sequence/type attrs,
                   columns, primary key, indexes, FKs, triggers
  SchemaDdlDiff    buildTableDdlDiffLines + the status-coloured renderer
  DetailTabs       the canonical ids/labels — Schema Blueprint, DDL Diff,
                   Migration SQL

Store wiring (deploy checkboxes, search highlight, the Monaco definition
editor) stays in the owning panel and is passed down as optional props or a
slot; omitting a handler hides its control rather than branching on a readOnly
flag.

Three surfaces render them now: Compare Schema, the history compare modal, and
the graph inspector. The inspector needed a TableDiff it did not have, so
inspectObject narrows both version states to the owner subtree and runs the
same CompareModule — one object's worth of compare, not the whole schema.
Direction matches diffVersions: source is the newer state, so ADDED reads as
"this version added it".

Also in this change: shape-level storage dedup (migration 14), per-object
change kinds on the graph, container growth roadmap, and selective revert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…robe holes

Four defects found while reviewing the SQL Editor, the data grid and the API
surface.

**Run ignored the caret.** With nothing checked and nothing selected, Run always
sent statement 1. An editor holding a page of queries would silently re-run the
top SELECT when the user pressed Run on the UPDATE further down — which reads
as "the editor won't run my UPDATE". Run now defaults to the statement under
the caret, falling back to the first when the editor has never been focused.
The button's tooltip names the cell (`In [3] at the caret`) so the label cannot
disagree with what executes.

**Write confirmation could execute a different statement than it listed.**
Resuming after the Safe-mode dialog re-derived the statements from the editor,
so a caret or checkbox change between "Run" and "Confirm" would swap them. The
resolved indices are now pinned into the pending confirmation.

**Wrong-row writes on case-colliding columns.** Data-grid column lookup folds
case, so two result columns differing only by case — Postgres and Db2 both
allow "ID" beside "id" — collapse onto one index and the last wins. A key
column could then read its value from the wrong column and the WHERE clause
match a different row. Editing is refused instead of guessed.

**CSV formula injection (CWE-1236).** Result grids export whatever the queried
database holds, and every major spreadsheet executes a cell starting with =, +,
-, @ or a tab. Both export paths — the .csv download and the clipboard TSV,
which is evaluated the same way on paste — now prefix a single quote. Plain
numbers are exempt: quoting -5 would turn numeric columns into text and break
every SUM in the file.

**POST /db/test had no restriction.** Its comment said the route was limited to
the local edition "because a connection probe would be an SSRF vector"; nothing
implemented that. The handler dials any host:port a caller names and reports
through the error text whether something answered, on a route with no
permission check. Now gated on isLocalSingleUser(), read per call so tests and
deployments both see the real value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot 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_6de2feac-1392-4b93-b77d-c644a367a900)

@huyplb
huyplb merged commit 85929e5 into main Aug 16, 2026
11 checks passed
@huyplb
huyplb deleted the feat/shared-schema-blueprint branch August 16, 2026 14:38
huyplb added a commit that referenced this pull request Aug 16, 2026
#259 landed the shared blueprint work first, which touched the same three
files. Resolved by keeping each side's better half:

- TopToolbar: main's comment explaining why the connection grid is hidden in
  History, plus this branch's <HistoryCompareBar /> in its place.
- LokeeWeavePage: main's compact one-line header — the tall title + subtitle +
  three-line totals card cost ~90px above the graph.
- LokeeWeaveView: this branch's store-backed version state wins. Main had grown
  a local useState pair and an inline picker bar for the same job; both are
  gone, so the sides live in lokeeHistoryStore and the picker is
  HistoryCompareBar alone. What stays from main is the compare *trigger* and
  the VersionCompareModal wiring, now reading the resolved pair out of the
  store — the bar chooses the versions, the button opens the diff for them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant