fix: report nested workspace conflicts
This commit is contained in:
@@ -254,7 +254,9 @@ export class WorkspaceRegistry {
|
||||
remote: unknown,
|
||||
prefix = "",
|
||||
): string[] {
|
||||
if (!base || !remote) return ["workspace.id"];
|
||||
if (base === undefined || remote === undefined) {
|
||||
return base === remote ? [] : [prefix || "workspace.id"];
|
||||
}
|
||||
if (Array.isArray(base) || Array.isArray(remote) || typeof base !== "object" || typeof remote !== "object") {
|
||||
return JSON.stringify(base) === JSON.stringify(remote) ? [] : [prefix];
|
||||
}
|
||||
|
||||
@@ -39,6 +39,97 @@ llm_policy:
|
||||
allowed: [zai/glm-5.2]
|
||||
`;
|
||||
|
||||
function withDwhRestTransport(source: string): string {
|
||||
return source.replace(
|
||||
"supported_transports: [postgres_direct]",
|
||||
"supported_transports: [postgres_direct, rest_api]",
|
||||
);
|
||||
}
|
||||
|
||||
function withDwhRestDiagnostic(source: string): string {
|
||||
return withDwhRestTransport(source).concat(`diagnostics:
|
||||
dwh_rest:
|
||||
method: GET
|
||||
path: /health
|
||||
auth: none
|
||||
response:
|
||||
database: database
|
||||
schema: schema
|
||||
`);
|
||||
}
|
||||
|
||||
function withEmbeddingDiagnostic(source: string): string {
|
||||
return source.concat(`diagnostics:
|
||||
embedding:
|
||||
method: GET
|
||||
path: /models
|
||||
auth: none
|
||||
response:
|
||||
model: model
|
||||
dimensions: dimensions
|
||||
`);
|
||||
}
|
||||
|
||||
function withDwhRestAndEmbeddingDiagnostics(source: string): string {
|
||||
return withDwhRestTransport(source).concat(`diagnostics:
|
||||
dwh_rest:
|
||||
method: GET
|
||||
path: /health
|
||||
auth: none
|
||||
response:
|
||||
database: database
|
||||
schema: schema
|
||||
embedding:
|
||||
method: GET
|
||||
path: /models
|
||||
auth: none
|
||||
response:
|
||||
model: model
|
||||
dimensions: dimensions
|
||||
`);
|
||||
}
|
||||
|
||||
function withVectorRestTransport(source: string): string {
|
||||
return source.replace(
|
||||
"supported_transports: [pgvector_direct]",
|
||||
"supported_transports: [pgvector_direct, rest_api]",
|
||||
);
|
||||
}
|
||||
|
||||
function withVectorMetadataDiagnostic(source: string): string {
|
||||
return withVectorRestTransport(source).concat(`diagnostics:
|
||||
vector_rest:
|
||||
metadata:
|
||||
method: GET
|
||||
path: /metadata
|
||||
auth: none
|
||||
response:
|
||||
collection: collection
|
||||
dimensions: dimensions
|
||||
distance: distance
|
||||
`);
|
||||
}
|
||||
|
||||
function withReversibleVectorProbe(source: string): string {
|
||||
return withVectorRestTransport(source).concat(`diagnostics:
|
||||
vector_rest:
|
||||
metadata:
|
||||
method: GET
|
||||
path: /metadata
|
||||
auth: none
|
||||
response:
|
||||
collection: collection
|
||||
dimensions: dimensions
|
||||
distance: distance
|
||||
reversible_probe:
|
||||
method: POST
|
||||
path: /probe
|
||||
auth: bearer
|
||||
response:
|
||||
operation: operation
|
||||
`);
|
||||
}
|
||||
|
||||
const runFile = promisify(execFile);
|
||||
const temporaryRoots: string[] = [];
|
||||
|
||||
@@ -237,6 +328,32 @@ test("reports stale publish conflicts with expected and actual revisions", async
|
||||
});
|
||||
});
|
||||
|
||||
test.each([
|
||||
["adds", withEmbeddingDiagnostic(withDwhRestTransport(validYaml)), withDwhRestAndEmbeddingDiagnostics(validYaml), "diagnostics.dwh_rest"],
|
||||
["removes", withDwhRestAndEmbeddingDiagnostics(validYaml), withEmbeddingDiagnostic(withDwhRestTransport(validYaml)), "diagnostics.dwh_rest"],
|
||||
["adds", withVectorMetadataDiagnostic(validYaml), withReversibleVectorProbe(validYaml), "diagnostics.vector_rest.reversible_probe"],
|
||||
["removes", withReversibleVectorProbe(validYaml), withVectorMetadataDiagnostic(validYaml), "diagnostics.vector_rest.reversible_probe"],
|
||||
])("reports an optional diagnostics branch when the registry %s it", async (_operation, baseSource, remoteSource, field) => {
|
||||
const remote = await fixture(baseSource);
|
||||
const registry = new WorkspaceRegistry(config(join(remote.root, "registry"), remote.remote));
|
||||
await registry.bootstrap();
|
||||
const initial = await registry.read("psd-clinical");
|
||||
writeFileSync(join(remote.source, "workspaces", "psd-clinical.yaml"), remoteSource);
|
||||
await git(remote.source, ["add", "workspaces/psd-clinical.yaml"]);
|
||||
await git(remote.source, ["commit", "-m", `Registry ${_operation} diagnostic branch`]);
|
||||
await git(remote.source, ["push", "origin", "main"]);
|
||||
|
||||
await expect(registry.publish({
|
||||
action: "update",
|
||||
workspace: workspaceWith("psd-clinical", { description: "Local stale change" }),
|
||||
baseCommit: initial.revision.commit,
|
||||
baseBlob: initial.revision.blob,
|
||||
})).rejects.toMatchObject({
|
||||
code: "workspace_conflict",
|
||||
fields: [field],
|
||||
});
|
||||
});
|
||||
|
||||
test("restores a clean checkout after a failed commit and retries publication", async () => {
|
||||
const remote = await fixture();
|
||||
const root = join(remote.root, "registry");
|
||||
|
||||
@@ -50,6 +50,41 @@ const diagnosticConflictFields = [
|
||||
"diagnostics.embedding.auth", "diagnostics.embedding.response.model", "diagnostics.embedding.response.dimensions",
|
||||
] as const;
|
||||
|
||||
const optionalDiagnosticsConflictFields = [
|
||||
"diagnostics",
|
||||
"diagnostics.dwh_rest",
|
||||
"diagnostics.vector_rest",
|
||||
"diagnostics.vector_rest.reversible_probe",
|
||||
"diagnostics.embedding",
|
||||
] as const;
|
||||
|
||||
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" } },
|
||||
},
|
||||
};
|
||||
|
||||
test.each(optionalDiagnosticsConflictFields)("accepts optional diagnostics conflict branch %s", async (field) => {
|
||||
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: workspace, 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({ fields: [field] });
|
||||
});
|
||||
|
||||
test.each(diagnosticConflictFields)("accepts canonical diagnostic conflict leaf %s with its remote revision", async (field) => {
|
||||
const diagnosticsWorkspace: CanonicalWorkspace = {
|
||||
...workspace,
|
||||
|
||||
@@ -142,6 +142,7 @@ const conflictFields = new Set([
|
||||
"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", "diagnostics.dwh_rest", "diagnostics.vector_rest", "diagnostics.vector_rest.reversible_probe", "diagnostics.embedding",
|
||||
"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",
|
||||
|
||||
@@ -30,6 +30,18 @@ const conflict: WorkspaceConflict = {
|
||||
remote: { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "remote-model" } } },
|
||||
};
|
||||
|
||||
const diagnosticsBranchConflict: WorkspaceConflict = {
|
||||
...conflict,
|
||||
fields: ["diagnostics.dwh_rest"],
|
||||
base: workspace,
|
||||
remote: workspace,
|
||||
local: {
|
||||
...workspace,
|
||||
dwh: { ...workspace.dwh, supported_transports: ["postgres_direct", "rest_api"] },
|
||||
diagnostics: { dwh_rest: { method: "GET", path: "/health", auth: "none", response: { database: "database", schema: "schema" } } },
|
||||
},
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
server.use(http.post("http://localhost:8787/workspaces/validate", () => HttpResponse.json({ workspace, contract: {} })));
|
||||
});
|
||||
@@ -101,6 +113,26 @@ test("saves explicit local choices as a rebased draft and does not republish it"
|
||||
expect(publishCalls).toBe(1);
|
||||
});
|
||||
|
||||
test("rebases a selected optional diagnostics branch into the revised draft", async () => {
|
||||
const user = userEvent.setup();
|
||||
const saved = vi.fn();
|
||||
server.use(http.post("http://localhost:8787/workspaces/publish", () => HttpResponse.json({
|
||||
...diagnosticsBranchConflict, 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 diagnostics.dwh_rest" }));
|
||||
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({ diagnostics: diagnosticsBranchConflict.local.diagnostics }),
|
||||
}));
|
||||
});
|
||||
|
||||
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({
|
||||
|
||||
+37
-1
@@ -39,6 +39,20 @@
|
||||
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.
|
||||
|
||||
## Review round 2
|
||||
|
||||
- Fixed recursive registry diffs so add/remove changes to optional nested diagnostics branches
|
||||
report their actual canonical paths instead of the fallback `workspace.id`. The regression cases
|
||||
cover both add and remove for `diagnostics.dwh_rest` and
|
||||
`diagnostics.vector_rest.reversible_probe`.
|
||||
- Extended the conflict-path allowlist to accept the optional `diagnostics` root and every
|
||||
optional diagnostics branch. The existing structural rebase now saves an explicitly selected
|
||||
branch (including an added or removed branch) in the revised browser draft, still pinned to the
|
||||
registry's actual revision and requiring normal validation and confirmation before publishing.
|
||||
- Wrote the backend/frontend cases first and observed the expected RED failures: backend conflict
|
||||
fields were `workspace.id`, while the frontend rejected the safe conflict payload before the
|
||||
resolution UI could render.
|
||||
|
||||
## Verification
|
||||
|
||||
Run in `frontend/` after the final changes:
|
||||
@@ -53,5 +67,27 @@ npx tsc -b
|
||||
|
||||
```text
|
||||
npx vitest run
|
||||
# 51 files passed, 392 tests passed
|
||||
# 51 files passed, 398 tests passed
|
||||
```
|
||||
|
||||
Round-2 focused verification:
|
||||
|
||||
```text
|
||||
backend: npx vitest run test/workspace-registry.test.ts
|
||||
# 1 file passed, 23 tests passed
|
||||
|
||||
backend: npx tsc --noEmit -p .
|
||||
# exit 0
|
||||
|
||||
frontend: npx vitest run
|
||||
# 51 files passed, 398 tests passed
|
||||
|
||||
frontend: npx tsc -b
|
||||
# exit 0
|
||||
```
|
||||
|
||||
The full backend `npx vitest run` was also attempted after allowing its local SSE test socket.
|
||||
The Task 10 registry tests passed, but seven unchanged SSE/session tests fail because their
|
||||
default, unbootstrapped registry makes session authorization return the intentional
|
||||
`session storage is unavailable` response. This failure is outside the Task 10 diff; it persists
|
||||
without any changed Task 10 route or test-harness code.
|
||||
|
||||
Reference in New Issue
Block a user