fix: resolve workspace publish conflicts
This commit is contained in:
@@ -38,3 +38,41 @@ test("rejects a conflict payload that attempts to surface a secret field", async
|
||||
|
||||
expect(asWorkspaceConflict(error)).toBeUndefined();
|
||||
});
|
||||
|
||||
const diagnosticConflictFields = [
|
||||
"diagnostics.dwh_rest.method", "diagnostics.dwh_rest.path", "diagnostics.dwh_rest.auth",
|
||||
"diagnostics.dwh_rest.response.database", "diagnostics.dwh_rest.response.schema",
|
||||
"diagnostics.vector_rest.metadata.method", "diagnostics.vector_rest.metadata.path", "diagnostics.vector_rest.metadata.auth",
|
||||
"diagnostics.vector_rest.metadata.response.collection", "diagnostics.vector_rest.metadata.response.dimensions",
|
||||
"diagnostics.vector_rest.metadata.response.distance", "diagnostics.vector_rest.reversible_probe.method",
|
||||
"diagnostics.vector_rest.reversible_probe.path", "diagnostics.vector_rest.reversible_probe.auth",
|
||||
"diagnostics.vector_rest.reversible_probe.response.operation", "diagnostics.embedding.method", "diagnostics.embedding.path",
|
||||
"diagnostics.embedding.auth", "diagnostics.embedding.response.model", "diagnostics.embedding.response.dimensions",
|
||||
] as const;
|
||||
|
||||
test.each(diagnosticConflictFields)("accepts canonical diagnostic conflict leaf %s with its remote revision", async (field) => {
|
||||
const diagnosticsWorkspace: CanonicalWorkspace = {
|
||||
...workspace,
|
||||
dwh: { ...workspace.dwh, supported_transports: ["postgres_direct", "rest_api"] },
|
||||
semantic_index: { ...workspace.semantic_index, vector_store: { ...workspace.semantic_index.vector_store, supported_transports: ["pgvector_direct", "rest_api"] } },
|
||||
diagnostics: {
|
||||
dwh_rest: { method: "POST", path: "/rpc/ping", auth: "bearer", response: { database: "database", schema: "schema" } },
|
||||
vector_rest: {
|
||||
metadata: { method: "GET", path: "/vector/metadata", auth: "bearer", response: { collection: "collection", dimensions: "dimensions", distance: "distance" } },
|
||||
reversible_probe: { method: "POST", path: "/vector/probe", auth: "x-api-key", response: { operation: "operation" } },
|
||||
},
|
||||
embedding: { method: "GET", path: "/models", auth: "none", response: { model: "model", dimensions: "dimensions" } },
|
||||
},
|
||||
};
|
||||
server.use(http.post("http://localhost:8787/workspaces/publish", () => HttpResponse.json({
|
||||
code: "workspace_conflict", message: "Workspace changed in the registry.",
|
||||
fields: [field],
|
||||
expected: { commit: "a".repeat(40), blob: "b".repeat(40) },
|
||||
actual: { commit: "c".repeat(40), blob: "d".repeat(40) },
|
||||
base: diagnosticsWorkspace, local: diagnosticsWorkspace, remote: diagnosticsWorkspace,
|
||||
}, { status: 409 })));
|
||||
|
||||
const error = await publishWorkspace({ action: "update", workspace: diagnosticsWorkspace, baseCommit: "a".repeat(40), baseBlob: "b".repeat(40) }).catch((cause: unknown) => cause);
|
||||
|
||||
expect(asWorkspaceConflict(error)).toMatchObject({ actual: { commit: "c".repeat(40), blob: "d".repeat(40) }, fields: [field] });
|
||||
});
|
||||
|
||||
@@ -115,11 +115,18 @@ export type PublishWorkspaceRequest =
|
||||
export interface WorkspaceConflict {
|
||||
code: "workspace_conflict";
|
||||
fields: string[];
|
||||
expected: WorkspaceConflictRevision;
|
||||
actual: WorkspaceConflictRevision;
|
||||
base: CanonicalWorkspace;
|
||||
local: CanonicalWorkspace;
|
||||
remote: CanonicalWorkspace;
|
||||
}
|
||||
|
||||
export interface WorkspaceConflictRevision {
|
||||
commit: string;
|
||||
blob?: string;
|
||||
}
|
||||
|
||||
export interface WorkspaceApiError {
|
||||
status: number;
|
||||
code: WorkspaceErrorCode;
|
||||
@@ -128,13 +135,21 @@ export interface WorkspaceApiError {
|
||||
}
|
||||
|
||||
const conflictFields = new Set([
|
||||
"workspace.name", "workspace.description", "workspace.language",
|
||||
"dwh.database", "dwh.schema", "dwh.port", "dwh.timeout_ms", "dwh.supported_transports",
|
||||
"semantic_index.vector_store.database", "semantic_index.vector_store.schema", "semantic_index.vector_store.collection",
|
||||
"workspace.schema_version", "workspace.id", "workspace.name", "workspace.description", "workspace.language",
|
||||
"dwh.engine", "dwh.database", "dwh.schema", "dwh.port", "dwh.timeout_ms", "dwh.supported_transports",
|
||||
"semantic_index.vector_store.engine", "semantic_index.vector_store.database", "semantic_index.vector_store.schema", "semantic_index.vector_store.collection",
|
||||
"semantic_index.vector_store.dimensions", "semantic_index.vector_store.distance", "semantic_index.vector_store.port",
|
||||
"semantic_index.vector_store.timeout_ms", "semantic_index.vector_store.supported_transports",
|
||||
"semantic_index.embedding.provider", "semantic_index.embedding.model", "semantic_index.embedding.dimensions",
|
||||
"semantic_index.embedding.timeout_ms", "semantic_index.vector_writer", "llm_policy.default", "llm_policy.allowed",
|
||||
"diagnostics.dwh_rest.method", "diagnostics.dwh_rest.path", "diagnostics.dwh_rest.auth",
|
||||
"diagnostics.dwh_rest.response.database", "diagnostics.dwh_rest.response.schema",
|
||||
"diagnostics.vector_rest.metadata.method", "diagnostics.vector_rest.metadata.path", "diagnostics.vector_rest.metadata.auth",
|
||||
"diagnostics.vector_rest.metadata.response.collection", "diagnostics.vector_rest.metadata.response.dimensions",
|
||||
"diagnostics.vector_rest.metadata.response.distance", "diagnostics.vector_rest.reversible_probe.method",
|
||||
"diagnostics.vector_rest.reversible_probe.path", "diagnostics.vector_rest.reversible_probe.auth",
|
||||
"diagnostics.vector_rest.reversible_probe.response.operation", "diagnostics.embedding.method", "diagnostics.embedding.path",
|
||||
"diagnostics.embedding.auth", "diagnostics.embedding.response.model", "diagnostics.embedding.response.dimensions",
|
||||
]);
|
||||
|
||||
const workspaceErrorCodes = new Set<WorkspaceErrorCode>([
|
||||
@@ -149,6 +164,15 @@ function object(value: unknown): Record<string, unknown> | undefined {
|
||||
: undefined;
|
||||
}
|
||||
|
||||
function conflictRevision(value: unknown): WorkspaceConflictRevision | undefined {
|
||||
const source = object(value);
|
||||
const commit = source?.commit;
|
||||
const blob = source?.blob;
|
||||
if (typeof commit !== "string" || !/^[0-9a-f]{40}$/.test(commit)) return undefined;
|
||||
if (blob !== undefined && (typeof blob !== "string" || !/^[0-9a-f]{40}$/.test(blob))) return undefined;
|
||||
return { commit, ...(typeof blob === "string" ? { blob } : {}) };
|
||||
}
|
||||
|
||||
/** Sanitized registry error data; it intentionally excludes the raw response body. */
|
||||
export function asWorkspaceApiError(error: unknown): WorkspaceApiError | undefined {
|
||||
if (!(error instanceof ApiError)) return undefined;
|
||||
@@ -173,10 +197,14 @@ export function asWorkspaceConflict(error: unknown): WorkspaceConflict | undefin
|
||||
const base = payload && sanitizeCanonicalWorkspace(payload.base);
|
||||
const local = payload && sanitizeCanonicalWorkspace(payload.local);
|
||||
const remote = payload && sanitizeCanonicalWorkspace(payload.remote);
|
||||
if (!payload || !fields || !fields.every((field) => conflictFields.has(field)) || !base || !local || !remote) return undefined;
|
||||
const expected = payload && conflictRevision(payload.expected);
|
||||
const actual = payload && conflictRevision(payload.actual);
|
||||
if (!payload || !fields || !fields.every((field) => conflictFields.has(field)) || !expected || !actual || !base || !local || !remote) return undefined;
|
||||
return {
|
||||
code: "workspace_conflict",
|
||||
fields,
|
||||
expected,
|
||||
actual,
|
||||
base,
|
||||
local,
|
||||
remote,
|
||||
|
||||
@@ -46,6 +46,7 @@ test("lists registry workspaces and saves a new workspace only as a browser draf
|
||||
|
||||
expect(await screen.findByRole("heading", { name: "Workspace management" })).toBeVisible();
|
||||
expect(await screen.findByRole("button", { name: "PSD Clinical" })).toBeVisible();
|
||||
expect(screen.getByText(`Commit ${"a".repeat(12)}`)).toBeVisible();
|
||||
await user.click(screen.getByRole("button", { name: "New workspace" }));
|
||||
await user.clear(screen.getByLabelText("Workspace ID"));
|
||||
await user.type(screen.getByLabelText("Workspace ID"), "trial-registry");
|
||||
@@ -124,6 +125,38 @@ test("stages duplicate and delete operations without publishing", async () => {
|
||||
expect(published).toBe(false);
|
||||
});
|
||||
|
||||
test("saves resolved conflict choices as a rebased browser draft without publishing again", async () => {
|
||||
const user = userEvent.setup();
|
||||
let publishCalls = 0;
|
||||
const local = { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "local-model" } } };
|
||||
const remote = { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "remote-model" } } };
|
||||
server.use(
|
||||
http.post("http://localhost:8787/workspaces/validate", () => HttpResponse.json({ workspace: local, contract: {} })),
|
||||
http.post("http://localhost:8787/workspaces/publish", () => {
|
||||
publishCalls += 1;
|
||||
return HttpResponse.json({
|
||||
code: "workspace_conflict", message: "Workspace changed in the registry.", fields: ["semantic_index.embedding.model"],
|
||||
expected: { commit: "a".repeat(40), blob: "b".repeat(40) }, actual: { commit: "c".repeat(40), blob: "d".repeat(40) },
|
||||
base: workspace, local, remote,
|
||||
}, { status: 409 });
|
||||
}),
|
||||
);
|
||||
renderManager();
|
||||
|
||||
await user.click(await screen.findByRole("button", { name: "PSD Clinical" }));
|
||||
await user.click(screen.getByRole("button", { name: "Publish draft" }));
|
||||
await user.click(screen.getByRole("button", { name: "Validate draft" }));
|
||||
await user.click(await screen.findByRole("button", { name: "Publish" }));
|
||||
await user.click(screen.getByRole("button", { name: "Confirm publish" }));
|
||||
await user.click(await screen.findByRole("radio", { name: "Use your draft for semantic_index.embedding.model" }));
|
||||
await user.click(screen.getByRole("button", { name: "Save revised draft" }));
|
||||
|
||||
expect(await screen.findByText("Revised draft saved with registry revision cccccccccccc. Validate it before publishing.")).toBeVisible();
|
||||
expect(localStorage.getItem("thothii.workspace-registry.v1.draft.psd-clinical")).toContain('"baseCommit":"cccccccccccccccccccccccccccccccccccccccc"');
|
||||
expect(localStorage.getItem("thothii.workspace-registry.v1.draft.psd-clinical")).toContain('"baseBlob":"dddddddddddddddddddddddddddddddddddddddd"');
|
||||
expect(publishCalls).toBe(1);
|
||||
});
|
||||
|
||||
test("proposes a different valid ID when duplicating a 63-character workspace ID", async () => {
|
||||
const user = userEvent.setup();
|
||||
const maxId = `w${"a".repeat(62)}`;
|
||||
|
||||
@@ -234,6 +234,16 @@ export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: ()
|
||||
void Promise.all([statusQuery.refetch(), workspacesQuery.refetch(), detailQuery.refetch()]);
|
||||
}
|
||||
|
||||
function saveResolvedDraft(draft: WorkspaceDraft) {
|
||||
workspaceDrafts.save(draft);
|
||||
setSelectedId(draft.workspaceId);
|
||||
setLocalDraft(draft);
|
||||
setDeletionDraft(undefined);
|
||||
setPublishRequest(undefined);
|
||||
setDiagnostics([]);
|
||||
setNotice(`Revised draft saved with registry revision ${draft.baseCommit.slice(0, 12)}. Validate it before publishing.`);
|
||||
}
|
||||
|
||||
async function validateCurrent() {
|
||||
if (!currentDraft) return;
|
||||
setNotice(undefined);
|
||||
@@ -295,7 +305,7 @@ export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: ()
|
||||
</div>
|
||||
<div className="border-t border-border/70 pt-3 text-xs text-muted-foreground">
|
||||
<p className="font-semibold text-foreground">Git status</p>
|
||||
{statusQuery.isError ? <QueryError name="Workspace registry status failed" message="Could not load registry status." retryLabel="Retry registry status" onRetry={() => { void statusQuery.refetch(); }} /> : statusQuery.isLoading ? <p>Loading…</p> : status && <><p>{status.branch}</p><p>{status.degraded ? "Degraded" : "Current"} · ↑{status.ahead} ↓{status.behind}</p></>}
|
||||
{statusQuery.isError ? <QueryError name="Workspace registry status failed" message="Could not load registry status." retryLabel="Retry registry status" onRetry={() => { void statusQuery.refetch(); }} /> : statusQuery.isLoading ? <p>Loading…</p> : status && <><p>{status.branch}</p>{status.head && <p className="font-mono">Commit {status.head.slice(0, 12)}</p>}<p>{status.degraded ? "Degraded" : "Current"} · ↑{status.ahead} ↓{status.behind}</p></>}
|
||||
</div>
|
||||
</nav>
|
||||
|
||||
@@ -333,7 +343,7 @@ export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: ()
|
||||
</div>
|
||||
</div>
|
||||
</DialogContent>
|
||||
{publishRequest && <WorkspacePublishDialog open={Boolean(publishRequest)} request={publishRequest} onOpenChange={(nextOpen) => { if (!nextOpen) setPublishRequest(undefined); }} onPublished={published} onPull={pullLatest} onReload={reloadWorkspace} />}
|
||||
{publishRequest && <WorkspacePublishDialog open={Boolean(publishRequest)} request={publishRequest} onOpenChange={(nextOpen) => { if (!nextOpen) setPublishRequest(undefined); }} onPublished={published} onResolved={saveResolvedDraft} onPull={pullLatest} onReload={reloadWorkspace} />}
|
||||
</Dialog>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -23,6 +23,8 @@ const request: PublishWorkspaceRequest = {
|
||||
const conflict: WorkspaceConflict = {
|
||||
code: "workspace_conflict",
|
||||
fields: ["semantic_index.embedding.model"],
|
||||
expected: { commit: "a".repeat(40), blob: "b".repeat(40) },
|
||||
actual: { commit: "c".repeat(40), blob: "d".repeat(40) },
|
||||
base: workspace,
|
||||
local: { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "local-model" } } },
|
||||
remote: { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "remote-model" } } },
|
||||
@@ -40,7 +42,7 @@ test("validates a draft and requires a separate confirmation before publishing",
|
||||
publishCalls += 1;
|
||||
return HttpResponse.json({ revision: { id: "psd-clinical", commit: "c".repeat(40), blob: "d".repeat(40), snapshotPath: "/safe", state: "operational" } });
|
||||
}));
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={published} onPull={vi.fn()} onReload={vi.fn()} />);
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={published} onResolved={vi.fn()} onPull={vi.fn()} onReload={vi.fn()} />);
|
||||
|
||||
expect(screen.getByRole("button", { name: "Publish" })).toBeDisabled();
|
||||
await user.click(screen.getByRole("button", { name: "Validate draft" }));
|
||||
@@ -61,7 +63,7 @@ test("shows a field-level conflict and never overwrites the remote workspace", a
|
||||
published = true;
|
||||
return HttpResponse.json({ ...conflict, message: "Workspace changed in the registry." }, { status: 409 });
|
||||
}));
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={vi.fn()} onPull={vi.fn()} onReload={vi.fn()} />);
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={vi.fn()} onResolved={vi.fn()} onPull={vi.fn()} onReload={vi.fn()} />);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Validate draft" }));
|
||||
await user.click(await screen.findByRole("button", { name: "Publish" }));
|
||||
@@ -70,18 +72,41 @@ test("shows a field-level conflict and never overwrites the remote workspace", a
|
||||
expect(await screen.findByText("semantic_index.embedding.model")).toBeVisible();
|
||||
expect(screen.getByText("local-model")).toBeVisible();
|
||||
expect(screen.getByText("remote-model")).toBeVisible();
|
||||
expect(screen.getByRole("button", { name: "Pull latest registry" })).toBeVisible();
|
||||
expect(screen.getByRole("button", { name: "Reload workspace" })).toBeVisible();
|
||||
expect(screen.queryByRole("button", { name: /use local|use remote|confirm publish/i })).not.toBeInTheDocument();
|
||||
expect(screen.getByRole("radio", { name: "Use your draft for semantic_index.embedding.model" })).toBeVisible();
|
||||
expect(screen.getByRole("radio", { name: "Use registry value for semantic_index.embedding.model" })).toBeVisible();
|
||||
expect(screen.getByRole("button", { name: "Save revised draft" })).toBeDisabled();
|
||||
expect(published).toBe(true);
|
||||
});
|
||||
|
||||
test("saves explicit local choices as a rebased draft and does not republish it", async () => {
|
||||
const user = userEvent.setup();
|
||||
const saved = vi.fn();
|
||||
let publishCalls = 0;
|
||||
server.use(http.post("http://localhost:8787/workspaces/publish", () => {
|
||||
publishCalls += 1;
|
||||
return HttpResponse.json({ ...conflict, message: "Workspace changed in the registry." }, { status: 409 });
|
||||
}));
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={vi.fn()} onResolved={saved} onPull={vi.fn()} onReload={vi.fn()} />);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Validate draft" }));
|
||||
await user.click(await screen.findByRole("button", { name: "Publish" }));
|
||||
await user.click(screen.getByRole("button", { name: "Confirm publish" }));
|
||||
await user.click(await screen.findByRole("radio", { name: "Use your draft for semantic_index.embedding.model" }));
|
||||
await user.click(screen.getByRole("button", { name: "Save revised draft" }));
|
||||
|
||||
expect(saved).toHaveBeenCalledWith(expect.objectContaining({
|
||||
baseCommit: "c".repeat(40), baseBlob: "d".repeat(40),
|
||||
workspace: expect.objectContaining({ semantic_index: expect.objectContaining({ embedding: expect.objectContaining({ model: "local-model" }) }) }),
|
||||
}));
|
||||
expect(publishCalls).toBe(1);
|
||||
});
|
||||
|
||||
test("keeps a conflict open and redacts a failed registry pull", async () => {
|
||||
const user = userEvent.setup();
|
||||
server.use(http.post("http://localhost:8787/workspaces/publish", () => HttpResponse.json({
|
||||
...conflict, message: "Workspace changed in the registry.",
|
||||
}, { status: 409 })));
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={vi.fn()} onPull={vi.fn().mockRejectedValue(new Error("token=secret"))} onReload={vi.fn()} />);
|
||||
render(<WorkspacePublishDialog open request={request} onOpenChange={vi.fn()} onPublished={vi.fn()} onResolved={vi.fn()} onPull={vi.fn().mockRejectedValue(new Error("token=secret"))} onReload={vi.fn()} />);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Validate draft" }));
|
||||
await user.click(await screen.findByRole("button", { name: "Publish" }));
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
type WorkspaceConflict,
|
||||
type WorkspaceRevision,
|
||||
} from "../api/workspaces";
|
||||
import type { WorkspaceDraft } from "../workspaces/drafts";
|
||||
import { Button } from "../components/ui/button";
|
||||
import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from "../components/ui/dialog";
|
||||
|
||||
@@ -17,6 +18,7 @@ export interface WorkspacePublishDialogProps {
|
||||
request: PublishWorkspaceRequest;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
onPublished: (revision: WorkspaceRevision | undefined) => void;
|
||||
onResolved: (draft: WorkspaceDraft) => void;
|
||||
onPull: () => Promise<void> | void;
|
||||
onReload: () => void;
|
||||
}
|
||||
@@ -30,6 +32,25 @@ function valueAt(workspace: CanonicalWorkspace, path: string): string {
|
||||
: JSON.stringify(value);
|
||||
}
|
||||
|
||||
function valueAtPath(workspace: CanonicalWorkspace, path: string): unknown {
|
||||
return path.split(".").reduce<unknown>((current, key) => (
|
||||
current && typeof current === "object" ? (current as Record<string, unknown>)[key] : undefined
|
||||
), workspace);
|
||||
}
|
||||
|
||||
function replaceAtPath(value: Record<string, unknown>, path: string[], replacement: unknown): Record<string, unknown> {
|
||||
const [key, ...remaining] = path;
|
||||
const copy = { ...value };
|
||||
if (remaining.length === 0) {
|
||||
if (replacement === undefined) delete copy[key];
|
||||
else copy[key] = replacement;
|
||||
return copy;
|
||||
}
|
||||
const child = copy[key];
|
||||
copy[key] = replaceAtPath(child && typeof child === "object" && !Array.isArray(child) ? child as Record<string, unknown> : {}, remaining, replacement);
|
||||
return copy;
|
||||
}
|
||||
|
||||
function requestWithWorkspace(request: PublishWorkspaceRequest, workspace: CanonicalWorkspace): PublishWorkspaceRequest {
|
||||
return request.action === "delete" ? request : { ...request, workspace };
|
||||
}
|
||||
@@ -39,10 +60,17 @@ function RevisionSummary({ request }: { request: PublishWorkspaceRequest }) {
|
||||
return <p className="rounded-md border border-border/70 bg-muted/40 px-3 py-2 font-mono text-xs text-muted-foreground">{label} · base {request.baseCommit.slice(0, 12)}</p>;
|
||||
}
|
||||
|
||||
function ConflictReview({ conflict, onPull, onReload }: { conflict: WorkspaceConflict; onPull: () => Promise<void> | void; onReload: () => void }) {
|
||||
function ConflictReview({ conflict, onPull, onReload, onResolved }: {
|
||||
conflict: WorkspaceConflict;
|
||||
onPull: () => Promise<void> | void;
|
||||
onReload: () => void;
|
||||
onResolved: (draft: WorkspaceDraft) => void;
|
||||
}) {
|
||||
const [pulling, setPulling] = useState(false);
|
||||
const [pulled, setPulled] = useState(false);
|
||||
const [pullError, setPullError] = useState(false);
|
||||
const [choices, setChoices] = useState<Record<string, "local" | "remote">>({});
|
||||
const canSave = Boolean(conflict.actual.blob) && conflict.fields.every((field) => choices[field]);
|
||||
|
||||
async function pull() {
|
||||
setPulling(true);
|
||||
@@ -57,10 +85,26 @@ function ConflictReview({ conflict, onPull, onReload }: { conflict: WorkspaceCon
|
||||
}
|
||||
}
|
||||
|
||||
function saveRevisedDraft() {
|
||||
if (!conflict.actual.blob || !canSave) return;
|
||||
const workspace = conflict.fields.reduce<CanonicalWorkspace>((current, field) => (
|
||||
choices[field] === "local"
|
||||
? replaceAtPath(current as unknown as Record<string, unknown>, field.split("."), valueAtPath(conflict.local, field)) as unknown as CanonicalWorkspace
|
||||
: current
|
||||
), conflict.remote);
|
||||
onResolved({
|
||||
workspaceId: workspace.workspace.id,
|
||||
baseCommit: conflict.actual.commit,
|
||||
baseBlob: conflict.actual.blob,
|
||||
workspace,
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
return <div className="grid gap-4">
|
||||
<div className="rounded-md border border-amber-500/30 bg-amber-500/10 px-3 py-2 text-sm">
|
||||
<p className="font-semibold">The registry changed before publication.</p>
|
||||
<p className="mt-1 text-muted-foreground">Your browser draft is unchanged. Pull or reload before creating a revised draft; this screen never merges or overwrites remote values.</p>
|
||||
<p className="mt-1 text-muted-foreground">Choose a value for every changed field to save a revised browser draft against registry revision {conflict.actual.commit.slice(0, 12)}. Saving never publishes it.</p>
|
||||
</div>
|
||||
<div className="grid gap-3">
|
||||
{conflict.fields.map((field) => <section key={field} className="overflow-hidden rounded-md border border-border/70" aria-label={`Conflict for ${field}`}>
|
||||
@@ -70,18 +114,24 @@ function ConflictReview({ conflict, onPull, onReload }: { conflict: WorkspaceCon
|
||||
<div className="min-w-0 px-3 py-2"><dt className="text-xs text-muted-foreground">Your draft</dt><dd className="mt-1 break-words font-mono text-xs">{valueAt(conflict.local, field)}</dd></div>
|
||||
<div className="min-w-0 px-3 py-2"><dt className="text-xs text-muted-foreground">Registry</dt><dd className="mt-1 break-words font-mono text-xs">{valueAt(conflict.remote, field)}</dd></div>
|
||||
</dl>
|
||||
<fieldset className="flex flex-wrap gap-3 border-t border-border/70 px-3 py-2 text-xs">
|
||||
<legend className="sr-only">Resolve {field}</legend>
|
||||
<label className="flex items-center gap-1.5"><input type="radio" name={`resolution-${field}`} aria-label={`Use your draft for ${field}`} checked={choices[field] === "local"} onChange={() => setChoices((current) => ({ ...current, [field]: "local" }))} />Use your draft</label>
|
||||
<label className="flex items-center gap-1.5"><input type="radio" name={`resolution-${field}`} aria-label={`Use registry value for ${field}`} checked={choices[field] === "remote"} onChange={() => setChoices((current) => ({ ...current, [field]: "remote" }))} />Use registry value</label>
|
||||
</fieldset>
|
||||
</section>)}
|
||||
</div>
|
||||
{pulled && <p role="status" className="text-sm text-muted-foreground">Latest registry state pulled. Reload the workspace before editing or publishing again.</p>}
|
||||
{pullError && <p role="alert" className="text-sm text-destructive">Registry pull could not be completed. Try again or reload the workspace.</p>}
|
||||
<div className="flex flex-wrap justify-end gap-2">
|
||||
<Button onClick={saveRevisedDraft} disabled={!canSave}>Save revised draft</Button>
|
||||
<Button variant="outline" onClick={() => { void pull(); }} disabled={pulling}>{pulling ? "Pulling…" : "Pull latest registry"}</Button>
|
||||
<Button onClick={onReload}>Reload workspace</Button>
|
||||
</div>
|
||||
</div>;
|
||||
}
|
||||
|
||||
export function WorkspacePublishDialog({ open, request, onOpenChange, onPublished, onPull, onReload }: WorkspacePublishDialogProps) {
|
||||
export function WorkspacePublishDialog({ open, request, onOpenChange, onPublished, onResolved, onPull, onReload }: WorkspacePublishDialogProps) {
|
||||
const [validatedRequest, setValidatedRequest] = useState<PublishWorkspaceRequest>();
|
||||
const [confirmationOpen, setConfirmationOpen] = useState(false);
|
||||
const [conflict, setConflict] = useState<WorkspaceConflict>();
|
||||
@@ -141,10 +191,10 @@ export function WorkspacePublishDialog({ open, request, onOpenChange, onPublishe
|
||||
<DialogContent showCloseButton={false} className="max-w-[min(94vw,46rem)] p-0">
|
||||
<DialogHeader className="border-b border-border/70 px-5 pb-4 pt-5 pr-12">
|
||||
<DialogTitle>{conflict ? "Publication conflict" : "Publish workspace"}</DialogTitle>
|
||||
<DialogDescription>{conflict ? "Compare the changed fields, then pull and reload before revising your draft." : "Validation and an explicit confirmation are required before this shared definition is published."}</DialogDescription>
|
||||
<DialogDescription>{conflict ? "Choose each local or registry value, then save a revised draft for normal validation and confirmation." : "Validation and an explicit confirmation are required before this shared definition is published."}</DialogDescription>
|
||||
</DialogHeader>
|
||||
<div className="grid gap-4 px-5 py-5">
|
||||
{conflict ? <ConflictReview conflict={conflict} onPull={onPull} onReload={onReload} /> : <>
|
||||
{conflict ? <ConflictReview conflict={conflict} onPull={onPull} onReload={onReload} onResolved={onResolved} /> : <>
|
||||
<RevisionSummary request={validatedRequest ?? request} />
|
||||
{message && <p role="status" className="rounded-md border border-border/70 bg-muted/40 px-3 py-2 text-sm">{message}</p>}
|
||||
<div className="flex flex-wrap justify-end gap-2">
|
||||
|
||||
+17
-3
@@ -16,6 +16,17 @@
|
||||
Conflict payloads now pass through the canonical draft sanitizer and reject unknown/secret
|
||||
fields before rendering.
|
||||
|
||||
## Review round 1
|
||||
|
||||
- Replaced the pull/reload-only conflict recovery with an explicit choice of the local draft or
|
||||
registry value for every changed field. A revised draft can be saved only after every field has
|
||||
a choice; it is rebased to the conflict's `actual.commit` and `actual.blob` and is never
|
||||
published automatically.
|
||||
- Kept normal validation and the explicit publish confirmation as mandatory steps after saving a
|
||||
resolution. Nothing silently discards the local draft or merges it into the registry.
|
||||
- Added typed expected/actual conflict revisions, displayed the active registry `status.head`
|
||||
commit, and whitelisted every canonical `diagnostics.*` leaf path structurally.
|
||||
|
||||
## TDD evidence
|
||||
|
||||
- Wrote the publish-dialog and manager import/export tests before the implementation and observed
|
||||
@@ -24,14 +35,17 @@
|
||||
before wiring the conflict parser through the canonical sanitizer.
|
||||
- Added a regression test for a failed pull during conflict recovery and observed the original
|
||||
unhandled rejection before adding the redacted in-dialog error state.
|
||||
- Added review-round tests first for per-field local/registry selection, rebased draft saving
|
||||
without a second publish, active-commit rendering, and every canonical diagnostics conflict
|
||||
path; these initially failed against the pull/reload-only UI and narrow path parser.
|
||||
|
||||
## Verification
|
||||
|
||||
Run in `frontend/` after the final changes:
|
||||
|
||||
```text
|
||||
npx vitest run src/shell/WorkspacePublishDialog.test.tsx src/api/workspaces.test.ts src/shell/WorkspaceManager.test.tsx
|
||||
# 3 files passed, 15 tests passed
|
||||
npx vitest run src/shell/WorkspacePublishDialog.test.tsx src/api/workspaces.test.ts src/shell/WorkspaceManager.test.tsx src/workspaces/drafts.test.ts
|
||||
# 4 files passed, 47 tests passed
|
||||
|
||||
npx tsc -b
|
||||
# exit 0
|
||||
@@ -39,5 +53,5 @@ npx tsc -b
|
||||
|
||||
```text
|
||||
npx vitest run
|
||||
# 51 files passed, 370 tests passed
|
||||
# 51 files passed, 392 tests passed
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user