From 508b34023d408fdda64261f71f18bc8d8aba316e Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Jul 2026 00:12:13 +0300 Subject: [PATCH 1/2] =?UTF-8?q?Branch=20staging:=20create=20=E2=86=92=20wr?= =?UTF-8?q?ite=20=E2=86=92=20review=20=E2=86=92=20merge=20in=20the=20app?= =?UTF-8?q?=20chrome=20(P6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every notebook write previously landed straight on the graph's main branch — no review step, no undo. This adds session branch staging to the web app over the server's first-class branches. Client facade - createBranch/mergeBranch/deleteBranch/snapshot on Client. mergeBranch returns a discriminated result ({ok, outcome} | {ok:false, conflicts}) — conflicts are an EXPECTED review outcome, not an exception. SDK 0.7/0.8 never populate ConflictError.mergeConflicts (they read camelCase from a snake_case error body), so the facade falls back to the raw body. Switch = keyed remount with carried state - BranchedApp holds the session branch ABOVE the runtime; switching remounts RuntimeApp with a source targeting the branch (defaultTarget + ServerSource opts stay in lockstep) and the previous snapshot state as initialState, so selections survive. ?branch= follows via replaceState. The session branch is distinct from the DECLARED base (CLI --branch / injected config, never the URL param): a ?branch= reload lands back ON the working branch while merges still target the declared base. BranchBar (header chrome) - Branch dropdown (refreshes on open, so branches created elsewhere appear), inline create prefilled work- (409 = "exists" → switch to it), and on a working branch a Review & merge popover: table-level deltas from two /snapshot reads, two-step-confirmed merge (outcome toast; structured conflict list on 409 with the base untouched), delete-to-abandon. Merge never auto-deletes the branch. - Table deltas compare version + row_count + WRITER lineage: versions are per-lineage counters, so two diverged branches can collide numerically while contents differ — same numbers with a different last-writer render as "diverged" instead of a false "no changes". Tests 219 → (client 31, web 71 here). Verified live on the RustFS demo: branch isolation (writes stayed off main), delta review, fast-forward merge + delete, and a real divergent_update conflict rendering structured while main stayed intact; prod smoke read-only. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011TrhtF1SiJghJASMWECzBh --- packages/client/src/http.test.ts | 122 ++++++ packages/client/src/http.ts | 143 +++++++ packages/client/src/index.ts | 4 + packages/web/src/App.test.ts | 5 +- packages/web/src/App.tsx | 67 ++- packages/web/src/branch.test.ts | 80 ++++ packages/web/src/branch.ts | 78 ++++ .../web/src/components/BranchBar.test.tsx | 160 ++++++++ packages/web/src/components/BranchBar.tsx | 388 ++++++++++++++++++ packages/web/src/config.ts | 40 +- 10 files changed, 1075 insertions(+), 12 deletions(-) create mode 100644 packages/web/src/branch.test.ts create mode 100644 packages/web/src/branch.ts create mode 100644 packages/web/src/components/BranchBar.test.tsx create mode 100644 packages/web/src/components/BranchBar.tsx diff --git a/packages/client/src/http.test.ts b/packages/client/src/http.test.ts index 77ad6e6..ae99fb1 100644 --- a/packages/client/src/http.test.ts +++ b/packages/client/src/http.test.ts @@ -188,3 +188,125 @@ describe("Client (SDK-backed facade)", () => { expect(err.message).toMatch(RE_CONFLICT); }); }); + +describe("branch staging surface", () => { + it("createBranch posts and resolves; a 409 (name exists) surfaces as OmnigraphHttpError", async () => { + const ok = clientWith( + fetchReturning( + jsonResponse({ uri: "og://g", from: "main", name: "stage", actor_id: null }), + ), + ); + await expect(ok.createBranch("stage")).resolves.toBeUndefined(); + + const exists = clientWith( + fetchReturning( + jsonResponse({ error: "branch 'stage' already exists", code: "conflict" }, 409), + ), + ); + await expect(exists.createBranch("stage")).rejects.toMatchObject({ + name: "OmnigraphHttpError", + status: 409, + }); + }); + + it("mergeBranch maps the three success outcomes", async () => { + for (const outcome of ["already_up_to_date", "fast_forward", "merged"]) { + const client = clientWith( + fetchReturning( + jsonResponse({ source: "stage", target: "main", outcome, actor_id: null }), + ), + ); + await expect(client.mergeBranch("stage")).resolves.toEqual({ + ok: true, + outcome, + }); + } + }); + + it("mergeBranch turns a 409 with merge_conflicts into {ok:false, conflicts}", async () => { + const client = clientWith( + fetchReturning( + jsonResponse( + { + error: "merge conflicts", + code: "conflict", + merge_conflicts: [ + { + table_key: "node:Task", + row_id: "t1", + kind: "DivergentUpdate", + message: "status changed on both branches", + }, + { + table_key: "edge:AssignedTo", + row_id: null, + kind: "OrphanEdge", + message: "edge target deleted on main", + }, + ], + }, + 409, + ), + ), + ); + const result = await client.mergeBranch("stage", "main"); + expect(result).toEqual({ + ok: false, + conflicts: [ + { + table_key: "node:Task", + row_id: "t1", + kind: "DivergentUpdate", + message: "status changed on both branches", + }, + { + table_key: "edge:AssignedTo", + kind: "OrphanEdge", + message: "edge target deleted on main", + }, + ], + }); + }); + + it("deleteBranch resolves on 2xx", async () => { + const client = clientWith( + fetchReturning(jsonResponse({ uri: "og://g", name: "stage", actor_id: null })), + ); + await expect(client.deleteBranch("stage")).resolves.toBeUndefined(); + }); + + it("snapshot reshapes tables to {table_key, version, row_count}", async () => { + const client = clientWith( + fetchReturning( + jsonResponse({ + branch: "stage", + manifest_version: 7, + tables: [ + { + table_key: "node:Task", + table_path: "tables/task", + table_version: 16, + table_branch: "stage", + row_count: 12, + }, + { + table_key: "node:Comment", + table_path: "tables/comment", + table_version: 9, + table_branch: null, + row_count: 4, + }, + ], + }), + ), + ); + await expect(client.snapshot("stage")).resolves.toEqual({ + branch: "stage", + manifest_version: 7, + tables: [ + { table_key: "node:Task", version: 16, row_count: 12, writer: "stage" }, + { table_key: "node:Comment", version: 9, row_count: 4, writer: null }, + ], + }); + }); +}); diff --git a/packages/client/src/http.ts b/packages/client/src/http.ts index cf7d215..edb0f15 100644 --- a/packages/client/src/http.ts +++ b/packages/client/src/http.ts @@ -12,6 +12,7 @@ import { Omnigraph, + ConflictError, NetworkError, OmnigraphError, type QueryInput as SdkQueryInput, @@ -74,6 +75,43 @@ export interface BranchListOutput { branches: string[]; } +/** One structured conflict from an all-or-nothing merge attempt (409). */ +export interface MergeConflictInfo { + table_key: string; + row_id?: string; + kind: string; + message: string; +} + +/** + * Merge result as a discriminated union: conflicts are an EXPECTED outcome of + * reviewing staged work, not an exception — the target is untouched on + * conflict, so callers render `conflicts` and keep going. + */ +export type BranchMergeResult = + | { ok: true; outcome: "already_up_to_date" | "fast_forward" | "merged" } + | { ok: false; conflicts: MergeConflictInfo[] }; + +/** Per-table manifest state on a branch — the table-level delta source. */ +export interface SnapshotTableInfo { + table_key: string; + version: number; + row_count: number; + /** + * The branch lineage that last wrote this table (null = the graph's main + * lineage). Version numbers are per-lineage counters, so two DIVERGED + * branches can share a version with different contents — the writer is + * what disambiguates them. + */ + writer: string | null; +} + +export interface SnapshotOutput { + branch: string; + manifest_version: number; + tables: SnapshotTableInfo[]; +} + export type ParamKind = | "string" | "bool" @@ -304,6 +342,111 @@ export class Client { } } + /** + * Fork `name` off `from` (server default: main). A name collision surfaces + * as the usual 409 `OmnigraphHttpError` — callers that want "switch to the + * existing branch instead" match on `status === 409`. + */ + async createBranch( + name: string, + from?: string, + signal?: AbortSignal, + ): Promise { + this.requireGraph("/branches"); + try { + await this.og.branches.create( + { name, ...(from !== undefined ? { from } : {}) }, + signal ? { signal } : {}, + ); + } catch (e) { + throw toHttpError(e, "/branches"); + } + } + + /** + * Merge `source` into `target` (server default: main). Three-way and + * all-or-nothing: on conflict nothing is published and the structured + * conflict list comes back as `{ok: false}` rather than a throw. + */ + async mergeBranch( + source: string, + target?: string, + signal?: AbortSignal, + ): Promise { + this.requireGraph("/branches/merge"); + try { + const r = await this.og.branches.merge( + { source, ...(target !== undefined ? { target } : {}) }, + signal ? { signal } : {}, + ); + return { + ok: true, + outcome: r.outcome as "already_up_to_date" | "fast_forward" | "merged", + }; + } catch (e) { + if (e instanceof ConflictError) { + // SDK 0.7.0 reads `body?.mergeConflicts` but the server sends + // snake_case `merge_conflicts` (error bodies aren't camelized), so + // `e.mergeConflicts` is always undefined — fall back to the raw body. + const raw = (e.body as { merge_conflicts?: unknown } | undefined) + ?.merge_conflicts; + const entries = + e.mergeConflicts?.map((c) => ({ + table_key: c.tableKey, + ...(c.rowId != null ? { row_id: c.rowId } : {}), + kind: String(c.kind), + message: c.message, + })) ?? + (Array.isArray(raw) + ? (raw as Record[]).map((c) => ({ + table_key: String(c.table_key ?? ""), + ...(c.row_id != null ? { row_id: String(c.row_id) } : {}), + kind: String(c.kind ?? "conflict"), + message: String(c.message ?? ""), + })) + : undefined); + if (entries !== undefined) return { ok: false, conflicts: entries }; + } + throw toHttpError(e, "/branches/merge"); + } + } + + /** Delete a branch pointer. Idempotent server-side (missing = no-op). */ + async deleteBranch(name: string, signal?: AbortSignal): Promise { + this.requireGraph(`/branches/${name}`); + try { + await this.og.branches.delete(name, signal ? { signal } : {}); + } catch (e) { + throw toHttpError(e, `/branches/${name}`); + } + } + + /** + * Latest-commit manifest state of a branch: per-table version + row count. + * Two snapshots (branch vs its base) make the table-level change summary. + */ + async snapshot(branch?: string, signal?: AbortSignal): Promise { + this.requireGraph("/snapshot"); + try { + const r = await this.og.snapshot( + branch !== undefined ? { branch } : {}, + signal ? { signal } : {}, + ); + return { + branch: r.branch, + manifest_version: r.manifestVersion, + tables: r.tables.map((t) => ({ + table_key: t.tableKey, + version: t.tableVersion, + row_count: t.rowCount, + writer: t.tableBranch ?? null, + })), + }; + } catch (e) { + throw toHttpError(e, "/snapshot"); + } + } + async healthz(): Promise { try { await this.og.health(); diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index aa0f8bb..4b7f6c0 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -7,6 +7,10 @@ export { type MutateInput, type ChangeOutput, type BranchListOutput, + type BranchMergeResult, + type MergeConflictInfo, + type SnapshotOutput, + type SnapshotTableInfo, type ParamDescriptor, type ParamKind, type QueriesOutput, diff --git a/packages/web/src/App.test.ts b/packages/web/src/App.test.ts index 04b402d..15249ef 100644 --- a/packages/web/src/App.test.ts +++ b/packages/web/src/App.test.ts @@ -11,7 +11,10 @@ describe("buildConfig", () => { "http://127.0.0.1:5173/?server=http://example.test&token=tok&branch=review&graph=acme", ); const config = await buildConfig(); - expect(config.label).toBe("server: http://example.test · graph: acme · review"); + // The branch is session-switchable chrome (BranchBar), not part of the + // static label — it surfaces on config.branch instead. + expect(config.label).toBe("server: http://example.test · graph: acme"); + expect(config.branch).toBe("review"); // rawGq is off unless the explicit ?allowRawGq escape hatch is present. expect(config.source.capabilities().rawGq).toBe(false); expect(storage.get("omnigraph_token")).toBe("tok"); diff --git a/packages/web/src/App.tsx b/packages/web/src/App.tsx index 92a135e..014e041 100644 --- a/packages/web/src/App.tsx +++ b/packages/web/src/App.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useState } from "react"; +import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Responsive, WidthProvider, type Layout } from "react-grid-layout/legacy"; import { JSONUIProvider, Renderer } from "@json-render/react"; import { @@ -37,6 +37,7 @@ import { } from "@/components/ui/card"; import { Skeleton } from "@/components/ui/skeleton"; import { ToastProvider, useToastManager } from "@/components/ui/toast"; +import { BranchBar } from "./components/BranchBar.js"; import { cn } from "@/lib/utils"; import { CheckIcon, @@ -154,12 +155,64 @@ export function App(): React.ReactElement { ); } - return ; + return ; } -function RuntimeApp({ config }: { config: AppConfig }): React.ReactElement { +/** + * Session branch staging: the active branch is React state ABOVE the runtime. + * Switching remounts RuntimeApp (key) with a source/runtime targeting the new + * branch, carrying the previous `$state` (selections survive). The URL's + * `?branch=` follows so reload/share stays on the branch. + */ +function BranchedApp({ config }: { config: AppConfig }): React.ReactElement { + // Base = the DECLARED branch (never the URL param), so booting via + // `?branch=work-x` still offers "merge into ". + const baseBranch = config.declaredBranch ?? "main"; + const [sessionBranch, setSessionBranch] = useState(config.branch ?? baseBranch); + const carriedState = useRef | undefined>(undefined); + const switchBranch = useCallback( + (branch: string, state: Record) => { + carriedState.current = state; + const url = new URL(window.location.href); + if (branch === baseBranch) url.searchParams.delete("branch"); + else url.searchParams.set("branch", branch); + window.history.replaceState(null, "", url); + setSessionBranch(branch); + }, + [baseBranch], + ); + return ( + + ); +} + +function RuntimeApp({ + config, + branch, + baseBranch, + initialState, + onSwitchBranch, +}: { + config: AppConfig; + branch: string; + baseBranch: string; + initialState: Record | undefined; + onSwitchBranch: (branch: string, state: Record) => void; +}): React.ReactElement { const [runtime] = useState(() => - createNotebookRuntime({ notebook: config.notebook, source: config.source }), + createNotebookRuntime({ + notebook: config.notebook, + source: config.makeSource(branch), + defaultTarget: { branch }, + ...(initialState !== undefined ? { initialState } : {}), + }), ); const [snapshot, setSnapshot] = useState(() => runtime.getSnapshot(), @@ -426,6 +479,12 @@ function RuntimeApp({ config }: { config: AppConfig }): React.ReactElement { {savingLayout ? "Saving…" : "Save layout"} )} + onSwitchBranch(next, snapshot.state)} + /> + {menuOpen && ( + <> +
setMenuOpen(false)} /> +
+
    + {branches.map((name) => ( +
  • + +
    • + ))} +
+
+ {creating ? ( +
{ + e.preventDefault(); + void createBranch(); + }} + > + setNewName(e.target.value)} + aria-label="New branch name" + /> + +
+ ) : ( + + )} +
+
+ + )} +
+ + {/* Review & merge — only on a working branch */} + {onBranch && ( +
+ + {review !== null && ( + <> +
setReview(null)} /> +
+

+ {current} → {baseBranch} +

+ {review.phase === "loading" && ( +

+ comparing snapshots… +

+ )} + {review.phase === "error" && ( +

{review.message}

+ )} + {review.phase === "ready" && ( + <> + {review.deltas.length === 0 ? ( +

+ No changes yet — the branch matches {baseBranch}. +

+ ) : ( +
    + {review.deltas.map((d) => ( +
  • + {tableDisplayName(d.table)} + + + {d.diverged ? "diverged" : rowDeltaLabel(d.rowDelta)} + + + {d.diverged + ? `both at v${d.toVersion}` + : `${d.fromVersion !== undefined ? `v${d.fromVersion} → ` : "new · "}v${d.toVersion}`} + + +
    • + ))} +
+ )} +
+ + +
+ + )} + {review.phase === "merged" && ( + <> +

+ Merged ({review.outcome.replace(/_/g, " ")}). The branch still + exists — delete it? +

+
+ + +
+ + )} + {review.phase === "conflicts" && ( + <> +

+ Merge blocked — {review.conflicts.length} conflict + {review.conflicts.length === 1 ? "" : "s"} ({baseBranch} is untouched): +

+
    + {review.conflicts.map((c, i) => ( +
  • + {tableDisplayName(c.table_key)} + {c.row_id !== undefined && ( + /{c.row_id} + )} + {" — "} + {c.kind} {c.message} +
    • + ))} +
+ + )} +
+ + )} +
+ )} +
+ ); +} diff --git a/packages/web/src/config.ts b/packages/web/src/config.ts index 64b4c3e..b4b8cb1 100644 --- a/packages/web/src/config.ts +++ b/packages/web/src/config.ts @@ -14,6 +14,21 @@ export interface AppConfig { notebook: ReturnType; source: Source; label: string; + /** The shared HTTP facade — the BranchBar drives branch ops through it. */ + client: Client; + /** The boot read/write branch (URL → injected → undefined = server default). */ + branch: string | undefined; + /** + * The DECLARED base branch (CLI --branch / operator config — never the URL + * param): a `?branch=` reload lands you back ON your working branch, and + * merges still target the declared base. Undefined ⇒ main. + */ + declaredBranch: string | undefined; + /** + * Build a Source targeting `branch` (undefined = server default) with this + * session's client + rawGq capability — the branch-switch rebuild path. + */ + makeSource: (branch: string | undefined) => Source; /** * The committed layout sidecar (`.layout.json`), injected by the * `view` BFF — the base layer under any personal localStorage tweaks. Null @@ -91,8 +106,10 @@ export async function buildConfig(): Promise { "Server mode requires top-level `server:` or a `?server=` URL parameter.", ); } - const branch = - url.searchParams.get("branch") ?? injected.branch ?? undefined; + // The URL param is SESSION state (the branch you were working on); the + // injected value is the declared base the deployment targets. + const declaredBranch = injected.branch ?? undefined; + const branch = url.searchParams.get("branch") ?? declaredBranch; // rawGq is off by default (operator/production context); enable only via the // explicit `?allowRawGq` escape hatch (e.g. `view --allow-raw-gq` forwards it). const allowRawGq = @@ -118,13 +135,22 @@ export async function buildConfig(): Promise { ? pruneOverrides(normalizeOverrides(injected.layout), liveIds) : null; + const makeSource = (target: string | undefined): Source => + new ServerSource(client, { + ...(target ? { branch: target } : {}), + ...(allowRawGq ? { allowRawGq: true } : {}), + }); + return { notebook, - source: new ServerSource(client, { - ...(branch ? { branch } : {}), - ...(allowRawGq ? { allowRawGq: true } : {}), - }), - label: `server: ${server} · graph: ${graph}${branch ? ` · ${branch}` : ""}`, + source: makeSource(branch), + // Branch is session-switchable (BranchBar) — it lives in the chrome, not + // the static label. + label: `server: ${server} · graph: ${graph}`, + client, + branch, + declaredBranch, + makeSource, initialLayout, canPersistLayout: injected.canPersistLayout === true, }; From 688624d26b0c40abd98574ffe660f40c505e3585 Mon Sep 17 00:00:00 2001 From: andrew Date: Wed, 8 Jul 2026 00:21:36 +0300 Subject: [PATCH 2/2] Delta summary covers base-only tables (staged removals) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile: computeTableDeltas iterated only the branch's tables, so a table present on the base but gone on the branch vanished from the review summary — the merge could be disabled ("no changes") or a destructive removal omitted. The union is now walked: base-only tables appear as removed deltas (badge "removed", -N rows). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011TrhtF1SiJghJASMWECzBh --- packages/web/src/branch.test.ts | 11 +++++++++++ packages/web/src/branch.ts | 16 ++++++++++++++++ packages/web/src/components/BranchBar.tsx | 6 +++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/web/src/branch.test.ts b/packages/web/src/branch.test.ts index f10e21c..16d8437 100644 --- a/packages/web/src/branch.test.ts +++ b/packages/web/src/branch.test.ts @@ -47,6 +47,17 @@ describe("computeTableDeltas", () => { expect(computeTableDeltas(a, b)).toEqual([]); }); + it("reports base-only tables as staged removals (union, not branch-only)", () => { + const base = snap("main", [ + ["node:Task", 14, 10], + ["node:Comment", 8, 3], + ]); + const branch = snap("stage", [["node:Task", 14, 10]]); + expect(computeTableDeltas(base, branch)).toEqual([ + { table: "node:Comment", rowDelta: -3, fromVersion: 8, toVersion: 8, diverged: false, removed: true }, + ]); + }); + it("flags DIVERGED lineages: same version+rows but a different writer", () => { // Versions are per-lineage counters — two branches that each made one // edit to the same table collide numerically while contents differ. diff --git a/packages/web/src/branch.ts b/packages/web/src/branch.ts index 08c6c4a..6a150be 100644 --- a/packages/web/src/branch.ts +++ b/packages/web/src/branch.ts @@ -14,6 +14,8 @@ export interface TableDelta { /** Base table version (undefined when the table is new on the branch). */ fromVersion: number | undefined; toVersion: number; + /** True when the table exists on the base but is GONE on the branch. */ + removed?: boolean; /** * True when the numbers alone would say "equal" but the table was last * written by a DIFFERENT lineage — versions are per-lineage counters, so @@ -49,6 +51,20 @@ export function computeTableDeltas( diverged: numbersEqual && !writersEqual, }); } + // Base-only tables (present on base, gone on the branch) are staged + // removals — they must appear in the summary, not vanish from it. + const branchKeys = new Set(branch.tables.map((t) => t.table_key)); + for (const t of base.tables) { + if (branchKeys.has(t.table_key)) continue; + deltas.push({ + table: t.table_key, + rowDelta: -t.row_count, + fromVersion: t.version, + toVersion: t.version, + diverged: false, + removed: true, + }); + } return deltas.sort((a, b) => a.table.localeCompare(b.table)); } diff --git a/packages/web/src/components/BranchBar.tsx b/packages/web/src/components/BranchBar.tsx index a0d63e6..c8e7579 100644 --- a/packages/web/src/components/BranchBar.tsx +++ b/packages/web/src/components/BranchBar.tsx @@ -294,7 +294,11 @@ export function BranchBar({ {tableDisplayName(d.table)} - {d.diverged ? "diverged" : rowDeltaLabel(d.rowDelta)} + {d.removed + ? "removed" + : d.diverged + ? "diverged" + : rowDeltaLabel(d.rowDelta)} {d.diverged