From 63941b58396b5202be8c46428e65f4f607ae0f61 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Thu, 27 Aug 2026 13:06:43 +0000 Subject: [PATCH] fix(integrations): ignore JSON object key order in ownership --- src/integrations/ownership-policy.ts | 29 ++++++++++--- src/integrations/ownership.ts | 38 ++++++++++++++++- src/integrations/state.ts | 47 +++++++++++++++++--- src/integrations/writer.ts | 24 +++++++++-- structure/09_client-integrations.md | 13 ++++++ tests/integrations-state.test.ts | 41 ++++++++++++++++++ tests/integrations-writer.test.ts | 64 ++++++++++++++++++++++++++++ 7 files changed, 239 insertions(+), 17 deletions(-) diff --git a/src/integrations/ownership-policy.ts b/src/integrations/ownership-policy.ts index bb7f34efe43..508910955fd 100644 --- a/src/integrations/ownership-policy.ts +++ b/src/integrations/ownership-policy.ts @@ -12,7 +12,7 @@ import { type ManagedContribution, type ManagedFragment, } from "../clients/config-export"; -import { canonicalContribution, fingerprint } from "./ownership"; +import { canonicalContribution, fingerprint, semanticContribution } from "./ownership"; type JsonObject = Record; @@ -124,11 +124,10 @@ export function validRefreshablePaths( }); } -/** Fingerprint a contribution after removing only its explicitly refreshable paths. */ -export function protectedContributionFingerprint( +function contributionWithoutRefreshablePaths( contribution: ManagedContribution, refreshablePaths: readonly (readonly string[])[], -): string { +): ManagedContribution { const fragments = contribution.fragments.map(cloneFragment); for (const refreshablePath of refreshablePaths) { for (const fragment of fragments) { @@ -137,5 +136,25 @@ export function protectedContributionFingerprint( break; } } - return fingerprint(canonicalContribution({ ...contribution, fragments })); + return { ...contribution, fragments }; +} + +/** Fingerprint a contribution after removing only its explicitly refreshable paths. */ +export function protectedContributionFingerprint( + contribution: ManagedContribution, + refreshablePaths: readonly (readonly string[])[], +): string { + return fingerprint(canonicalContribution( + contributionWithoutRefreshablePaths(contribution, refreshablePaths), + )); +} + +/** Semantic protected fingerprint that ignores JSON object-key order only. */ +export function semanticProtectedContributionFingerprint( + contribution: ManagedContribution, + refreshablePaths: readonly (readonly string[])[], +): string { + return fingerprint(semanticContribution( + contributionWithoutRefreshablePaths(contribution, refreshablePaths), + )); } diff --git a/src/integrations/ownership.ts b/src/integrations/ownership.ts index 15e8234350f..537e53fd646 100644 --- a/src/integrations/ownership.ts +++ b/src/integrations/ownership.ts @@ -22,8 +22,38 @@ export function fingerprint(text: string): string { } /** - * Canonical bytes of a contribution. Fragments are sorted by path so two builds - * of the same contribution hash identically regardless of emission order. + * Canonicalize JSON object members recursively for semantic comparisons. + * Arrays stay ordered because their position can carry configuration meaning. + */ +function semanticJsonValue(value: unknown): unknown { + if (Array.isArray(value)) return value.map(semanticJsonValue); + if (value === null || typeof value !== "object") return value; + + const record = value as Record; + return Object.fromEntries( + Object.keys(record) + .sort() + .map(key => [key, semanticJsonValue(record[key])]), + ); +} + +/** + * Stable semantic bytes of a contribution. Third-party clients may + * re-serialize JSON object members in a different order; that formatting-only + * rewrite must not look like a protected-value edit. + */ +export function semanticContribution(contribution: ManagedContribution): string { + const sorted = [...contribution.fragments].sort((a, b) => { + const left = a.path.join("\u0000"); + const right = b.path.join("\u0000"); + return left < right ? -1 : left > right ? 1 : 0; + }); + return JSON.stringify(sorted.map(fragment => [fragment.path, semanticJsonValue(fragment.value)])); +} + +/** + * Legacy-compatible bytes used by existing persisted fingerprints. Fragment + * paths are stable, while nested object insertion order remains exact. */ export function canonicalContribution(contribution: ManagedContribution): string { const sorted = [...contribution.fragments].sort((a, b) => { @@ -41,11 +71,15 @@ export interface OwnershipRecord { fileFingerprint: string; /** Hash of our contribution — detects catalog/port drift. */ blockFingerprint: string; + /** Key-order-independent companion for JSON clients that normalize objects. */ + semanticBlockFingerprint?: string; /** * Hash of the fields the client must not rewrite. Present only when a * client has explicitly declared runtime-derived paths below. */ protectedBlockFingerprint?: string; + /** Key-order-independent companion to `protectedBlockFingerprint`. */ + semanticProtectedBlockFingerprint?: string; /** * Exact document paths a client may derive after apply. These are recorded * per operation so later catalog changes cannot widen an older grant. diff --git a/src/integrations/state.ts b/src/integrations/state.ts index 550f32ffa81..2d3713a785d 100644 --- a/src/integrations/state.ts +++ b/src/integrations/state.ts @@ -12,10 +12,11 @@ import { ClientPathError, EXPORT_CLIENTS, opencodeProxyBaseUrl, type ExportModel import type { OcxConfig } from "../types"; import { PARSE_FAILED, loadTarget, parseConfig, type IntegrationIO } from "./config-io"; import { SNAPSHOT_RETENTION } from "./journal"; -import { canonicalContribution, fingerprint, type OwnershipRecord } from "./ownership"; +import { canonicalContribution, fingerprint, semanticContribution, type OwnershipRecord } from "./ownership"; import { protectedContributionFingerprint, refreshablePathsOf, + semanticProtectedContributionFingerprint, validRefreshablePaths, } from "./ownership-policy"; import { INTEGRATION_CLIENTS, type IntegrationClientId } from "./registry"; @@ -153,21 +154,49 @@ function recordedBlockIsOwned( if (!observed) return false; if (fingerprint(canonicalContribution(observed)) === record.blockFingerprint) return true; + const observedSemanticFingerprint = fingerprint(semanticContribution(observed)); + if ( + typeof record.semanticBlockFingerprint === "string" + && observedSemanticFingerprint === record.semanticBlockFingerprint + ) return true; + + const desiredFingerprint = fingerprint(canonicalContribution(desired)); + if ( + desiredFingerprint === record.blockFingerprint + && observedSemanticFingerprint === fingerprint(semanticContribution(desired)) + ) return true; + if ( typeof record.protectedBlockFingerprint === "string" && validRefreshablePaths(observed, record.refreshablePaths) && record.refreshablePaths.length > 0 ) { - return protectedContributionFingerprint(observed, record.refreshablePaths) - === record.protectedBlockFingerprint; + const observedProtectedFingerprint = protectedContributionFingerprint( + observed, + record.refreshablePaths, + ); + if (observedProtectedFingerprint === record.protectedBlockFingerprint) return true; + + const observedSemanticProtectedFingerprint = semanticProtectedContributionFingerprint( + observed, + record.refreshablePaths, + ); + if ( + typeof record.semanticProtectedBlockFingerprint === "string" + && observedSemanticProtectedFingerprint === record.semanticProtectedBlockFingerprint + ) return true; + + return protectedContributionFingerprint(desired, record.refreshablePaths) + === record.protectedBlockFingerprint + && observedSemanticProtectedFingerprint + === semanticProtectedContributionFingerprint(desired, record.refreshablePaths); } - const desiredFingerprint = fingerprint(canonicalContribution(desired)); if (desiredFingerprint !== record.blockFingerprint) return false; const legacyPaths = refreshablePathsOf(desired); return legacyPaths.length > 0 - && protectedContributionFingerprint(observed, legacyPaths) - === protectedContributionFingerprint(desired, legacyPaths); + && semanticProtectedContributionFingerprint(observed, legacyPaths) + === semanticProtectedContributionFingerprint(desired, legacyPaths); } /** @@ -257,7 +286,11 @@ export function classifyIntegration(input: { } return { state: "stale" }; } - return input.record.blockFingerprint === fingerprint(canonicalContribution(input.contribution)) + const desiredFingerprint = typeof input.record.semanticBlockFingerprint === "string" + ? fingerprint(semanticContribution(input.contribution)) + : fingerprint(canonicalContribution(input.contribution)); + const recordedFingerprint = input.record.semanticBlockFingerprint ?? input.record.blockFingerprint; + return recordedFingerprint === desiredFingerprint ? { state: "current" } : { state: "stale" }; } diff --git a/src/integrations/writer.ts b/src/integrations/writer.ts index 21cf18ac4d3..f8eb2903279 100644 --- a/src/integrations/writer.ts +++ b/src/integrations/writer.ts @@ -15,8 +15,18 @@ import { EXPORT_CLIENTS, type ExportModel, type ManagedContribution } from "../c import { isLoopbackHostname } from "../codex/inject"; import type { OcxConfig } from "../types"; import { PARSE_FAILED, defaultIntegrationIO, loadTarget, parseConfig, type IntegrationIO } from "./config-io"; -import { fingerprint, canonicalContribution, fragmentPathsOf, type OwnershipRecord } from "./ownership"; -import { protectedContributionFingerprint, refreshablePathsOf } from "./ownership-policy"; +import { + fingerprint, + canonicalContribution, + fragmentPathsOf, + semanticContribution, + type OwnershipRecord, +} from "./ownership"; +import { + protectedContributionFingerprint, + refreshablePathsOf, + semanticProtectedContributionFingerprint, +} from "./ownership-policy"; import { createdContainerPaths, mergeContribution, removeFragments } from "./merge"; import { INTEGRATION_CLIENTS, isLoopbackOnly, type IntegrationClientId } from "./registry"; import { classifyIntegration, exportContextOf } from "./state"; @@ -361,8 +371,13 @@ function applyOrRefreshIntegration(input: IntegrationWriteInput, allowAbsent: bo record: { clientId, configPath, fileFingerprint: fingerprint(text), blockFingerprint: fingerprint(canonicalContribution(contribution)), + semanticBlockFingerprint: fingerprint(semanticContribution(contribution)), ...(refreshablePaths.length > 0 ? { protectedBlockFingerprint: protectedContributionFingerprint(contribution, refreshablePaths), + semanticProtectedBlockFingerprint: semanticProtectedContributionFingerprint( + contribution, + refreshablePaths, + ), refreshablePaths, } : {}), fragmentPaths: fragmentPathsOf(contribution), createdContainers: created, @@ -556,7 +571,10 @@ export function restoreIntegration(input: IntegrationRestoreInput): WriteOutcome ? (restoredText === null ? "absent" : "conflict") : !recordDescribesBytes ? "conflict" - : restoredRecord.blockFingerprint === fingerprint(canonicalContribution(fresh)) + : ( + restoredRecord.semanticBlockFingerprint === fingerprint(semanticContribution(fresh)) + || restoredRecord.blockFingerprint === fingerprint(canonicalContribution(fresh)) + ) ? "current" : "stale"; diff --git a/structure/09_client-integrations.md b/structure/09_client-integrations.md index fd9e911900e..42f0f0df3ac 100644 --- a/structure/09_client-integrations.md +++ b/structure/09_client-integrations.md @@ -41,6 +41,19 @@ added only to a writer would let a mutation bypass the state users saw. `fileFingerprint` records the exact whole-file result for restore and for serializers that may lose comments. `blockFingerprint` records the exact generated contribution and detects catalog, model, port, or provider drift. `fragmentPaths` bounds disable to the paths OpenCodex actually created. +New records pair the exact contribution fingerprints with semantic fingerprints that recursively +sort JSON object keys while preserving array order. Existing records without the semantic companion +fall back to comparing the recorded generated contribution when the catalog has not moved. This +keeps old records readable while preventing a client's formatting-only key reorder from +masquerading as a protected edit. + +[Decision Log] +- 목적과 의도: Treat JSON object-key order as formatting while retaining safe ownership proof across upgrades. +- 기존 구현 및 제약 조건: Existing records contain order-sensitive hashes, and replacing their hash format in place would make every installed integration look foreign-edited. +- 검토한 주요 대안: Replace the hash format globally; ignore key order only for ZCode; store a semantic companion beside the existing exact hash. +- 선택한 방식: Preserve the exact hashes for compatibility and add object-key-independent semantic companions to new records, with a bounded desired-contribution fallback for old records. +- 다른 대안 대신 이 방식을 선택한 이유: A global replacement cannot validate old records, while a ZCode-only exception would leave the shared JSON ownership rule inconsistent. +- 장점, 단점 및 영향: New records tolerate key normalization even across catalog refreshes; old records recover when the recorded catalog is still reconstructible, and ambiguous old-record drift remains fail-closed. Clients normally protect every field in every recorded fragment. A client that writes documented, runtime-derived fields back into an owned fragment may additionally record: diff --git a/tests/integrations-state.test.ts b/tests/integrations-state.test.ts index bc6cf8f6625..040b5fa40a4 100644 --- a/tests/integrations-state.test.ts +++ b/tests/integrations-state.test.ts @@ -7,6 +7,7 @@ import { PARSE_FAILED, fileIO, loadTarget, parseConfig } from "../src/integratio import { serializeDocument } from "../src/integrations/serialize"; import { canonicalContribution, + semanticContribution, fingerprint, writeRecord, type OwnershipRecord, @@ -502,6 +503,46 @@ describe("classifier unit behavior", () => { const reversed = { ...contribution, fragments: [...contribution.fragments].reverse() }; expect(canonicalContribution(reversed)).toBe(canonicalContribution(contribution)); }); + + test("nested JSON object key order does not change the contribution fingerprint (#2759)", () => { + const original = { + clientId: "zcode" as const, + fragments: [{ + path: ["provider", "opencodex"], + value: { + enabled: true, + options: { apiKey: "loopback", baseURL: "http://127.0.0.1:10100/v1" }, + models: { + routed: { + modalities: { input: ["text", "image"], output: ["text"] }, + limit: { context: 350_000 }, + }, + }, + }, + }], + }; + const reordered = { + clientId: "zcode" as const, + fragments: [{ + path: ["provider", "opencodex"], + value: { + models: { + routed: { + limit: { context: 350_000 }, + modalities: { output: ["text"], input: ["text", "image"] }, + }, + }, + options: { baseURL: "http://127.0.0.1:10100/v1", apiKey: "loopback" }, + enabled: true, + }, + }], + }; + + expect(semanticContribution(reordered)).toBe(semanticContribution(original)); + const reorderedArray = structuredClone(reordered); + reorderedArray.fragments[0]!.value.models.routed.modalities.input = ["image", "text"]; + expect(semanticContribution(reorderedArray)).not.toBe(semanticContribution(original)); + }); }); describe("ownership is scoped to recorded fragments", () => { diff --git a/tests/integrations-writer.test.ts b/tests/integrations-writer.test.ts index dc0964432f6..875f75cfd8c 100644 --- a/tests/integrations-writer.test.ts +++ b/tests/integrations-writer.test.ts @@ -119,6 +119,16 @@ function input(overrides: Partial = {}): IntegrationWrite }; } +function reverseJsonObjectKeys(value: unknown): unknown { + if (Array.isArray(value)) return value.map(reverseJsonObjectKeys); + if (value === null || typeof value !== "object") return value; + return Object.fromEntries( + Object.entries(value as Record) + .reverse() + .map(([key, nested]) => [key, reverseJsonObjectKeys(nested)]), + ); +} + describe("apply", () => { test("refuses a client that is not installed, and writes nothing", () => { const result = applyIntegration(input()); @@ -216,6 +226,8 @@ describe("apply", () => { const record = store.readRecords().zcode!; expect(record.protectedBlockFingerprint).toMatch(/^[0-9a-f]{16}$/); + expect(record.semanticBlockFingerprint).toMatch(/^[0-9a-f]{16}$/); + expect(record.semanticProtectedBlockFingerprint).toMatch(/^[0-9a-f]{16}$/); expect(record.refreshablePaths).toContainEqual([ "provider", "opencodex", "models", "mystery/model", "limit", "context", ]); @@ -251,6 +263,58 @@ describe("apply", () => { expect(after.provider.opencodex!.models["mystery/model"]!.limit).toBeUndefined(); }); + test("ZCode key-order normalization stays refreshable with derived metadata (#2759)", () => { + const configPath = installZcode(); + const models: ExportModel[] = [ + ...MODELS, + { namespaced: "mystery/model", provider: "mystery", id: "model" }, + ]; + const request = input({ clientId: "zcode", models }); + expect(applyIntegration(request).ok).toBe(true); + + const document = JSON.parse(readFileSync(configPath, "utf8")) as { + provider: Record> }>; + }; + document.provider.opencodex!.models["mystery/model"]!.limit = { + context: 128_000, + output: 32_000, + }; + document.provider.opencodex!.models["mystery/model"]!.reasoning = { enabled: false }; + const reordered = reverseJsonObjectKeys(document); + writeFileSync(configPath, `${JSON.stringify(reordered, null, 2)}\n`); + + expect(readIntegrationState(request)).toMatchObject({ state: "stale" }); + const refreshed = applyIntegration(request); + expect(refreshed.ok).toBe(true); + if (refreshed.ok) expect(refreshed.changed).toBe(true); + expect(readIntegrationState(request)).toMatchObject({ state: "current" }); + }); + + test("legacy ZCode records tolerate key reordering when the catalog is unchanged (#2759)", () => { + const configPath = installZcode(); + const request = input({ clientId: "zcode" }); + expect(applyIntegration(request).ok).toBe(true); + + const legacy = { ...store.readRecords().zcode! }; + delete legacy.semanticBlockFingerprint; + delete legacy.semanticProtectedBlockFingerprint; + store.putRecord(legacy); + + const document = JSON.parse(readFileSync(configPath, "utf8")) as { + provider: Record> }>; + }; + document.provider.opencodex!.models["anthropic/claude-opus-4-8"]!.reasoning = { + enabled: true, + }; + writeFileSync( + configPath, + `${JSON.stringify(reverseJsonObjectKeys(document), null, 2)}\n`, + ); + + expect(readIntegrationState(request)).toMatchObject({ state: "stale" }); + expect(applyIntegration(request).ok).toBe(true); + }); + test("a legacy ZCode record accepts derived drift only while its generated catalog is unchanged (#2389)", () => { const configPath = installZcode(); const request = input({ clientId: "zcode" });