fix(integrations): classify a json sibling edit as stale instead of conflict

docs-site/src/content/docs/guides/integrations.md

@@ -65,7 +65,14 @@ always recoverable:
   and their history rows read **Backup expired**.
 
 Disable removes only the entries opencodex recorded as its own. If your file changed
-after we wrote it, the switch locks and disable refuses rather than guessing which
+after we wrote it, what happens depends on whether our own entries are still intact
+and on the file's format. For strict-JSON configs (OpenCode, Pi), an edit **next to**
+our block — adding an MCP server, a provider of your own — shows as **Update needed**:
+refreshing merges around your entries and keeps them, though formatting may be
+normalized. The exception is a value JSON cannot rewrite exactly — a non-finite
+number like `1e999`, an integer literal past 2^53, or `-0` — which locks the
+switch instead, so the value is never silently changed. For formats that can carry comments (YAML, JSON5, TOML), or when our own
+entries were edited, the switch locks and disable refuses rather than guessing which
 edits were yours.
 
 ## What to expect, honestly

docs-site/src/content/docs/tr/guides/integrations.md

@@ -77,9 +77,18 @@ sahip olduğunuz durum her zaman kurtarılabilir:
   expired)** yazar.
 
 Devre dışı bırakma, yalnızca opencodex'in kendisine ait olarak kaydettiği
-girdileri kaldırır. Dosyanız biz yazdıktan sonra değiştiyse, anahtar kilitlenir
-ve hangi düzenlemelerin size ait olduğunu tahmin etmek yerine devre dışı
-bırakmayı reddeder.
+girdileri kaldırır. Dosyanız biz yazdıktan sonra değiştiyse, ne olacağı kendi
+girdilerimizin hâlâ bozulmamış olup olmadığına ve dosyanın biçimine bağlıdır.
+Katı JSON yapılandırmalarında (OpenCode, Pi), bloğumuzun **yanında** yapılan bir
+düzenleme — bir MCP sunucusu eklemek, kendinize ait bir sağlayıcı tanımlamak —
+**Güncelleme gerekli (Update needed)** olarak görünür: yenileme, girdilerinizin
+etrafında birleştirir ve onları korur; yalnızca biçimlendirme
+normalleştirilebilir. İstisna, JSON'un birebir yeniden yazamayacağı bir değerdir
+— `1e999` gibi sonlu olmayan bir sayı, 2^53'ü aşan bir tam sayı sabiti veya `-0`
+— bu durumda anahtar kilitlenir, böylece değer hiçbir zaman sessizce
+değiştirilmez. Yorum taşıyabilen biçimlerde (YAML, JSON5, TOML) veya kendi
+girdilerimiz düzenlenmişse, anahtar kilitlenir ve hangi düzenlemelerin size ait
+olduğunu tahmin etmek yerine devre dışı bırakmayı reddeder.
 
 ## Dürüstçe ne beklenmeli?
 

docs-site/src/content/docs/zh-tw/guides/integrations.md

@@ -37,7 +37,7 @@ opencodex 從自己的環境讀取這些變數。如果你的 gateway 以 profil
 - **Restore this point…** 會出現在較舊的操作上，或當檔案在那次操作之後有變更時。跨過這樣的變更做回復會再詢問一次，才覆蓋你的較新編輯——並且也會備份它們，所以那次的回復本身也可以復原。
 - 每個客戶端保留十份備份。超過之後，最舊的快照檔案會被移除，其歷史列顯示為 **Backup expired**。
 
-停用只移除 opencodex 記錄為自己寫入的條目。如果你的檔案在我們寫入之後有變更，開關會鎖定，停用會拒絕執行，而不是猜測哪些編輯是你的。
+停用只移除 opencodex 記錄為自己寫入的條目。如果你的檔案在我們寫入之後有變更，後續行為取決於我們自己的條目是否完好，以及檔案的格式。對於嚴格 JSON 設定檔（OpenCode、Pi），在我們的區塊**旁邊**進行的編輯——例如新增 MCP 伺服器或你自己的 provider——會顯示為**需要更新**：重新整理會在保留你的條目的前提下合併寫入，但格式可能會被正規化。例外情況是 JSON 無法精確重寫的值——例如 `1e999` 這類非有限數字、超過 2^53 的整數字面值，或 `-0`——此時開關會鎖定，確保這些值永遠不會被悄悄改動。對於可以包含註解的格式（YAML、JSON5、TOML），或當我們自己的條目被編輯過時，開關會鎖定，停用會拒絕執行，而不是猜測哪些編輯是你的。
 
 ## 誠實的預期
 

src/integrations/config-io.ts

@@ -23,12 +23,65 @@ import type { IntegrationClientId } from "./registry";
  */
 export const PARSE_FAILED = Symbol("parse-failed");
 
+/**
+ * Number literals JSON.parse has already damaged: `1e999` overflows to
+ * Infinity (a later rewrite would bake in `null` — the merge layer's JSON
+ * clone does it even before the serializer could refuse), an integer literal
+ * beyond 2^53 may have been rounded (a rewrite then hands consumers that read
+ * JSON integers exactly — python, jq, BigInt revivers — a different value),
+ * and `-0` re-serializes as `0`. By the time the document is parsed the
+ * original literal is gone, which is why this scans the RAW text — same
+ * reasoning as the TOML inf/nan guard below — and only literals whose value
+ * actually changed: `1e21` or 2^54 round-trip exactly and stay usable.
+ * Scanning also avoids recursing over attacker-shaped nesting depth.
+ */
+function jsonNumberLiteralsRoundTrip(text: string): boolean {
+  let inString = false;
+  let escaped = false;
+  for (let i = 0; i < text.length; i += 1) {
+    const ch = text[i]!;
+    if (inString) {
+      if (escaped) escaped = false;
+      else if (ch === "\\") escaped = true;
+      else if (ch === "\"") inString = false;
+      continue;
+    }
+    if (ch === "\"") { inString = true; continue; }
+    if (ch !== "-" && (ch < "0" || ch > "9")) continue;
+    let end = i + 1;
+    while (end < text.length && /[0-9+\-.eE]/.test(text[end]!)) end += 1;
+    const literal = text.slice(i, end);
+    i = end - 1;
+    const value = Number(literal);
+    if (!Number.isFinite(value)) return false;
+    if (value === 0 && literal.startsWith("-")) return false;
+    /*
+     * Deliberately plain digit runs only. They are the one spelling real
+     * consumers read with exact integer semantics (python's json yields an
+     * arbitrary-precision int, jq preserves big integer literals), so baking
+     * in the rounded double changes what those consumers extract. Decimal or
+     * exponent spellings of the same value (`9007199254740993e0`, `…3.0`) are
+     * float semantics for every consumer — they round identically before and
+     * after a rewrite, and shortest-round-trip stringify preserves what any
+     * reader can observe, so refusing them would only manufacture dead ends
+     * (`1e308` is not exactly representable either, yet rewrites losslessly
+     * for every possible reader).
+     */
+    const digits = literal[0] === "-" ? literal.slice(1) : literal;
+    if (/^[0-9]{16,}$/.test(digits) && BigInt(literal) !== BigInt(value)) return false;
+  }
+  return true;
+}
+
 /** Parse a client config, tolerating absence. PARSE_FAILED on garbage. */
 export function parseConfig(text: string | null, format: ConfigFormat): unknown | typeof PARSE_FAILED {
   if (text === null || text.trim().length === 0) return {};
   try {
     switch (format) {
-      case "json": return JSON.parse(text);
+      case "json": {
+        const parsed = JSON.parse(text);
+        return jsonNumberLiteralsRoundTrip(text) ? parsed : PARSE_FAILED;
+      }
       case "json5": return Bun.JSON5.parse(text);
       case "yaml": return Bun.YAML.parse(text);
       case "toml": {

src/integrations/state.ts

@@ -134,9 +134,15 @@ function recordedFragmentFingerprint(
 /**
  * The two-axis rule: the recorded bytes or fragments prove nobody changed
  * what we may rewrite, and the contribution hash proves our catalog has not
- * moved on. OMP is the sole fragment-scoped client because its writer patches
- * only `providers.opencodex`; every whole-document serializer retains the
- * whole-file fingerprint guard.
+ * moved on. Three classes of client (revising the unconditional whole-file
+ * rule of devlog 260802_client_toggle_api/021 §3 for json — #1631):
+ * OMP is fragment-scoped because its writer patches only
+ * `providers.opencodex`, so the whole-file check is skipped entirely;
+ * strict-json clients keep the whole-file check but downgrade a drift with
+ * intact owned fragments to `stale`, because a rewrite there can lose only
+ * formatting (comments cannot parse, non-round-tripping numbers are refused
+ * by the serializer); every comment-capable whole-document serializer (yaml,
+ * json5, toml) retains the whole-file fingerprint guard as a hard conflict.
  */
 export function classifyIntegration(input: {
   fileText: string | null;
@@ -180,12 +186,36 @@ export function classifyIntegration(input: {
     return { state: "conflict", reason: "unowned-key" };
   }
   const clientId = input.clientId ?? input.record.clientId;
-  if (clientId !== "omp" && fingerprint(input.fileText ?? "") !== input.record.fileFingerprint) {
-    return { state: "conflict", reason: "foreign-edit" };
-  }
+  /*
+   * Checked BEFORE file-level drift: an edit INSIDE an owned fragment is a
+   * conflict no matter what the rest of the file looks like, so the sibling-
+   * edit exemption below can never mask it.
+   */
   if (recordedFragmentFingerprint(input.parsed, input.record) !== input.record.blockFingerprint) {
     return { state: "conflict", reason: "foreign-edit" };
   }
+  if (clientId !== "omp" && fingerprint(input.fileText ?? "") !== input.record.fileFingerprint) {
+    /*
+     * The file changed since we wrote it, but every fragment we own is still
+     * byte-for-byte what we put there — a sibling edit, not tampering. Apply
+     * rewrites the WHOLE document, so for comment-capable formats (yaml,
+     * json5, toml) it would drop comments the user wrote next to us: fail
+     * closed there. Strict JSON cannot carry comments — a commented file
+     * never reaches this branch because parsing already failed — so the only
+     * possible loss is formatting normalization: numbers that would not
+     * round-trip (non-finite, integers past 2^53) are PARSE_FAILED in
+     * parseConfig and classify as unsafe long before this branch, exactly
+     * like comments. Refusing forever over formatting
+     * dead-ends the integration on the user's first own config edit (#1631).
+     * Report drift instead; a re-apply merges into the parsed document as it
+     * stands and re-owns the file. This also lets disable proceed on a
+     * drifted file — removal still touches only the recorded fragment paths.
+     */
+    if (EXPORT_CLIENTS[clientId].format !== "json") {
+      return { state: "conflict", reason: "foreign-edit" };
+    }
+    return { state: "stale" };
+  }
   return input.record.blockFingerprint === fingerprint(canonicalContribution(input.contribution))
     ? { state: "current" }
     : { state: "stale" };

src/integrations/writer.ts

@@ -213,7 +213,8 @@ function preflight(input: IntegrationWriteInput) {
   const before = target.before;
   const parsed = parseConfig(before, exportSpec.format);
   if (parsed === PARSE_FAILED) {
-    return { failed: refuse(clientId, "unsafe", "unsafe", `${configPath} could not be parsed`) } as const;
+    return { failed: refuse(clientId, "unsafe", "unsafe",
+      `${configPath} could not be parsed, or holds a value opencodex cannot rewrite without changing it (a non-finite number, an integer past 2^53, or -0)`) } as const;
   }
   const contribution = exportSpec.buildContribution(exportContextOf(input));
   // A record proves ownership of the file it was written FOR. Matching only by
@@ -368,8 +369,15 @@ export function disableIntegration(input: IntegrationWriteInput): WriteOutcome {
         : `${configPath} cannot be changed safely`);
   }
 
-  // current | stale only: the file fingerprint still matches our record, so the
-  // recorded paths are exactly what we put there.
+  /*
+   * current | stale only. What makes the removal safe is the BLOCK
+   * fingerprint, not the file fingerprint: the classifier verified the values
+   * at the recorded paths are byte-for-byte what we wrote, so removing them
+   * cannot take a user edit with them. The file itself may have drifted — a
+   * json client classifies a sibling edit as stale (#1631) — which is why the
+   * removal runs against the document as parsed NOW, and the re-serialize is
+   * value-safe because non-round-tripping numbers were refused at parse time.
+   */
   const { doc, removed } = removeFragments(
     parsed,
     record!.fragmentPaths,

tests/integrations-serialize.test.ts

@@ -155,6 +155,10 @@ describe("serializeDocument", () => {
     expect(() => serializeDocument([1, 2], "toml")).toThrow(/TOML root must be a table/);
   });
 
+
+
+
+
   test("media types are declared for every format", () => {
     expect(Object.keys(FORMAT_MEDIA_TYPE).sort()).toEqual(["json", "json5", "toml", "yaml"]);
   });

tests/integrations-state.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { EXPORT_CLIENTS, type ExportModel } from "../src/clients/config-export";
@@ -128,6 +128,20 @@ describe("the five states, each triggered directly", () => {
     expect(readIntegrationState(input()).state).toBe("stale");
   });
 
+  test("stale: a sibling edit next to an intact json block is drift, not conflict", () => {
+    // The full readIntegrationState path — real file I/O, configPath and
+    // clientId guards engaged — not just the synthetic classify calls below.
+    seedRecord(seedOurConfig());
+    const path = join(home, ".pi", "agent", "models.json");
+    const edited = JSON.parse(readFileSync(path, "utf8")) as {
+      providers: Record<string, unknown>;
+    };
+    edited.providers.mine = { baseUrl: "http://user.invalid/v1" };
+    writeFileSync(path, `${JSON.stringify(edited, null, 2)}\n`);
+
+    expect(readIntegrationState(input()).state).toBe("stale");
+  });
+
   test("conflict: an owned fragment changed after we wrote it", () => {
     const text = seedOurConfig();
     seedRecord(text);
@@ -384,6 +398,49 @@ describe("classifier unit behavior", () => {
     expect(parseConfig("{{{", "json")).toBe(PARSE_FAILED);
   });
 
+  test("parseConfig refuses json number literals a rewrite would change", () => {
+    // Overflow to Infinity — a rewrite would bake in null.
+    expect(parseConfig("{\"a\": 1e999}", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("[-1e999]", "json")).toBe(PARSE_FAILED);
+    // Rounded at parse: consumers reading JSON integers exactly (python, jq)
+    // would see a different value after the rewrite.
+    expect(parseConfig("{\"a\": 9007199254740993}", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("{\"a\": [{\"b\": [9007199254740993]}]}", "json")).toBe(PARSE_FAILED);
+    // Negative zero re-serializes as 0 — in every literal spelling.
+    expect(parseConfig("{\"a\": -0}", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("{\"a\": -0.0}", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("{\"a\": -0e5}", "json")).toBe(PARSE_FAILED);
+    // Scanner lexing edges: a bare top-level literal (token touches both text
+    // boundaries), a literal right after a comma, and a backslash-terminated
+    // string followed by a real literal (escape-flag handling).
+    expect(parseConfig("9007199254740993", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("[1, 9007199254740993]", "json")).toBe(PARSE_FAILED);
+    expect(parseConfig("{\"a\": \"x\\\\\", \"b\": 9007199254740993}", "json")).toBe(PARSE_FAILED);  });
+
+  test("parseConfig keeps json numbers that round-trip exactly", () => {
+    // 1e21 and 2^54 are exactly representable doubles; only the literal's
+    // spelling may normalize, never the value any JSON consumer reads.
+    expect(parseConfig("{\"a\": 1e21}", "json")).toEqual({ a: 1e21 });
+    expect(parseConfig("{\"a\": 18014398509481984}", "json")).toEqual({ a: 2 ** 54 });
+    // A huge number inside a string is data, not a number literal — even
+    // behind an escaped quote.
+    expect(parseConfig("{\"a\": \"1e999\"}", "json")).toEqual({ a: "1e999" });
+    expect(parseConfig("{\"a\": \"id 9007199254740993 ok\"}", "json"))
+      .toEqual({ a: "id 9007199254740993 ok" });
+    expect(parseConfig("{\"a\": \"he said \\\" 9007199254740993\"}", "json"))
+      .toEqual({ a: "he said \" 9007199254740993" });
+    // Decimal/exponent spellings are float semantics for every consumer —
+    // they round identically before and after a rewrite, so they stay usable
+    // (only plain digit runs carry exact-integer semantics, e.g. python's
+    // json module reads them as arbitrary-precision int).
+    expect(parseConfig("{\"a\": 9007199254740993e0}", "json"))
+      .toEqual({ a: 9007199254740992 });
+    expect(parseConfig("{\"a\": 9007199254740993.0}", "json"))
+      .toEqual({ a: 9007199254740992 });
+    // The guard is json-only: json5 keeps today's behavior.
+    expect(parseConfig("{\"a\": 9007199254740993}", "json5")).toEqual({ a: 9007199254740992 });
+  });
+
   test("fragment order does not change the contribution fingerprint", () => {
     const reversed = { ...contribution, fragments: [...contribution.fragments].reverse() };
     expect(canonicalContribution(reversed)).toBe(canonicalContribution(contribution));
@@ -471,7 +528,11 @@ describe("ownership is scoped to recorded fragments", () => {
     expect(result).toEqual({ state: "conflict", reason: "foreign-edit" });
   });
 
-  test("whole-document serializers still conflict on an unrelated source edit", () => {
+  test("json clients report a sibling edit as stale, not conflict", () => {
+    // Strict JSON cannot carry comments — a commented file fails parsing long
+    // before this branch — so rewriting the document cannot destroy anything
+    // but formatting. Refusing forever here dead-ended the integration on the
+    // user's first own config edit (#1631).
     const piContribution = { ...ownedContribution, clientId: "pi" as const };
     const piRecord: OwnershipRecord = {
       ...record,
@@ -487,6 +548,62 @@ describe("ownership is scoped to recorded fragments", () => {
       contribution: piContribution,
     });
 
+    expect(result).toEqual({ state: "stale" });
+  });
+
+  // Re-serializing a whole document in these formats would drop any comments
+  // the user keeps next to our block, so file-level drift stays a hard
+  // conflict for every one of them — a regression that narrowed the condition
+  // (say, to yaml only) must fail here, not in a user's config.
+  for (const { clientId, configPath } of [
+    { clientId: "hermes" as const, configPath: "/tmp/hermes-config.yaml" },
+    { clientId: "openclaw" as const, configPath: "/tmp/openclaw.json5" },
+    { clientId: "kimi" as const, configPath: "/tmp/kimi-config.toml" },
+  ]) {
+    test(`${clientId} (comment-capable) still conflicts on an unrelated source edit`, () => {
+      const contribution = { ...ownedContribution, clientId };
+      const clientRecord: OwnershipRecord = {
+        ...record,
+        clientId,
+        configPath,
+        blockFingerprint: fingerprint(canonicalContribution(contribution)),
+      };
+      const result = classifyIntegration({
+        fileText: textWithExtra,
+        fileIsRegular: true,
+        parsed: documentWithExtra,
+        record: clientRecord,
+        contribution,
+      });
+
+      expect(result).toEqual({ state: "conflict", reason: "foreign-edit" });
+    });
+  }
+
+  test("a json sibling edit combined with an edit inside our block is still a conflict", () => {
+    // The sibling-edit exemption must never mask tampering with an owned
+    // fragment: the block check runs first.
+    const piContribution = { ...ownedContribution, clientId: "pi" as const };
+    const piRecord: OwnershipRecord = {
+      ...record,
+      clientId: "pi",
+      configPath: "/tmp/pi-models.json",
+      blockFingerprint: fingerprint(canonicalContribution(piContribution)),
+    };
+    const editedDocument = {
+      providers: {
+        opencodex: { ...ownedValue, baseUrl: "http://user-edited.invalid/v1" },
+        freebuff: extraValue,
+      },
+    };
+    const result = classifyIntegration({
+      fileText: `${JSON.stringify(editedDocument, null, 2)}\n`,
+      fileIsRegular: true,
+      parsed: editedDocument,
+      record: piRecord,
+      contribution: piContribution,
+    });
+
     expect(result).toEqual({ state: "conflict", reason: "foreign-edit" });
   });
 });