fix(mcp): preserve TOML multiline strings during removal

src/hosts/mcp-config.ts

@@ -112,24 +112,81 @@ function tomlHeader(name: Brand): string {
   return `[mcp_servers.${name}]`;
 }
 
+type TomlStringQuote = '"' | "'";
+
+/** Carry multiline string context across lines without interpreting their text as tables. */
+function nextTomlMultilineQuote(line: string, carried: TomlStringQuote | null): TomlStringQuote | null {
+  let quote = carried;
+  let multiline = carried !== null;
+  for (let i = 0; i < line.length; i++) {
+    const char = line[i];
+    if (quote !== null) {
+      if (quote === '"' && char === '\\') {
+        i++; // Basic strings escape the next character; literal strings do not.
+      } else if (char === quote) {
+        if (!multiline) {
+          quote = null;
+        } else if (line.startsWith(quote.repeat(3), i)) {
+          const closing = quote;
+          quote = null;
+          multiline = false;
+          i += 2;
+          // Four/five closing quotes include one/two literal quotes, not a new string.
+          while (line[i + 1] === closing) i++;
+        }
+      }
+    } else {
+      if (char === '#') break;
+      if (char === '"' || char === "'") {
+        quote = char;
+        multiline = line.startsWith(char.repeat(3), i);
+        if (multiline) i += 2;
+      }
+    }
+  }
+  return multiline ? quote : null;
+}
+
 /**
- * Remove the `[mcp_servers.graft]` table (or `name`'s) from a TOML config, returning the rest.
+ * Remove every canonical table owned by `name` from a TOML config, returning the rest.
  *
  * Line-based on purpose: a real parse-and-reserialize would reformat the user's
- * whole file. The table runs from its header to the next `[`-header or EOF, which
- * is exactly the shape {@link upsertCodexToml} appends. Exported so the writer and
+ * whole file. An owned table is `[mcp_servers.<name>]` itself or any dotted
+ * subtable below it (`[mcp_servers.<name>.*]`, which TOML scopes under the
+ * parent) — wherever it appears in the file, contiguous or not, and even when
+ * the parent is gone and only an orphaned subtable remains. Anything else
+ * starting a new table (`[mcp_servers.other]`, `[mcp_servers.graft2]`,
+ * `[other.graft]`) is foreign and preserved. Exported so the writer and
  * `retract.ts` can never disagree about what "graft's section" means.
  */
 export function stripTomlSection(text: string, name: Brand = 'graft'): { rest: string; found: boolean } {
-  const lines = text.split('\n');
-  const header = tomlHeader(name);
-  const start = lines.findIndex((l) => l.trim() === header);
-  if (start === -1) return { rest: text, found: false };
-  let end = start + 1;
-  while (end < lines.length && !lines[end].trimStart().startsWith('[')) end++;
-  const rest = [...lines.slice(0, start), ...lines.slice(end)]
-    .join('\n')
-    .replace(/\n{3,}/g, '\n\n')
+  const lines = text.split(/(?<=\n)/);
+  const ownedHeader = tomlHeader(name);
+  // The trailing dot keeps similarly-named tables (`[mcp_servers.graft2]`,
+  // `[other.graft]`) out of the family.
+  const familyPrefix = `${ownedHeader.slice(0, -1)}.`;
+  const isFamilyHeader = (line: string): boolean => {
+    const trimmed = line.trimStart();
+    if (!trimmed.startsWith('[')) return false;
+    const header = trimmed.split(/\s+/)[0];
+    return header === ownedHeader || header.startsWith(familyPrefix);
+  };
+  let multiline: TomlStringQuote | null = null;
+  let found = false;
+  let inOwned = false;
+  const kept: string[] = [];
+  for (const line of lines) {
+    if (multiline === null && line.trimStart().startsWith('[')) {
+      inOwned = isFamilyHeader(line);
+      found ||= inOwned;
+    }
+    if (!inOwned) kept.push(line);
+    multiline = nextTomlMultilineQuote(line, multiline);
+  }
+  if (!found) return { rest: text, found: false };
+  // Interior blank lines can be multiline string data; preserve them verbatim.
+  const rest = kept
+    .join('')
     .replace(/^\n+/, '');
   return { rest, found: true };
 }

test/hosts-mcp-config.test.ts

@@ -7,10 +7,42 @@ process.env.GRAFT_MCP_NPX = '1';
 import { mkdtempSync, mkdirSync, readFileSync, writeFileSync, existsSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
-import { registerMcpConfigs, serverEntry } from '../src/hosts/mcp-config.js';
+import { registerMcpConfigs, serverEntry, stripTomlSection } from '../src/hosts/mcp-config.js';
 
 function fresh(): string { return mkdtempSync(join(tmpdir(), 'graft-mcpcfg-')); }
 
+for (const name of ['graft', 'trail'] as const) {
+  for (const quote of ['"', "'"]) {
+    test(`${name} removal preserves multiline ${quote} data and the other brand`, () => {
+      const other = name === 'graft' ? 'trail' : 'graft';
+      const delimiter = quote.repeat(3);
+      const foreign = `[mcp_servers.${other}.env]\r\nSCRIPT = ${delimiter}\r\n[mcp_servers.${name}]\r\n\r\n\r\n[mcp_servers.${name}.env]\r\n${delimiter}\r\nKEEP = "yes"\r\n`;
+      const similar = `[mcp_servers.${name}2]\r\ncommand = "foreign"\r\n`;
+      const owned = `[mcp_servers.${name}]\r\nNOTE = ${delimiter}\r\n[mcp_servers.foreign]\r\n${delimiter}\r\n`;
+      const text = owned + foreign + `[mcp_servers.${name}.env]\r\nOWNED = "remove"\r\n` + similar;
+      assert.deepEqual(stripTomlSection(text, name), { found: true, rest: foreign + similar });
+      assert.deepEqual(stripTomlSection(foreign + similar, name), { found: false, rest: foreign + similar });
+    });
+  }
+}
+
+test('Trail registration migrates both real table families but preserves multiline lookalikes', () => {
+  const repo = fresh(); const home = fresh();
+  mkdirSync(join(home, '.codex'), { recursive: true });
+  const path = join(home, '.codex', 'config.toml');
+  const foreign = '[mcp_servers.other.env]\nSCRIPT = """\n[mcp_servers.graft]\n\n\n[mcp_servers.trail.env]\n"""\nKEEP = "yes"\n';
+  writeFileSync(path, '[mcp_servers.graft]\ncommand = "old"\n' + foreign
+    + '[mcp_servers.trail.env]\nOLD_TRAIL = "remove"\n[mcp_servers.graft.env]\nOLD_GRAFT = "remove"\n');
+  const writes = registerMcpConfigs(repo, ['agents'], { home, name: 'trail' });
+  assert.deepEqual(writes.map((w) => w.action), ['updated']);
+  const result = readFileSync(path, 'utf8');
+  assert.ok(result.startsWith(foreign), 'foreign multiline data is byte-identical');
+  assert.doesNotMatch(result, /OLD_TRAIL|OLD_GRAFT|command = "old"/);
+  assert.match(result, /^\[mcp_servers\.trail\]$/m);
+  assert.deepEqual(registerMcpConfigs(repo, ['agents'], { home, name: 'trail' }).map((w) => w.action), ['unchanged']);
+  assert.equal(readFileSync(path, 'utf8'), result);
+});
+
 test('cursor/gemini/kiro get repo-local JSON entries', () => {
   const repo = fresh(); const home = fresh();
   const w = registerMcpConfigs(repo, ['cursor', 'gemini', 'kiro'], { home });
@@ -110,3 +142,188 @@ test('GRAFT_MCP_NPX overrides an installed binary', () => {
   process.env.GRAFT_MCP_NPX = '1';
   assert.equal(serverEntry({ onPath: true }).command, 'npx', 'the escape hatch wins');
 });
+
+// ---------------------------------------------------------------------------
+// Issue #546: dotted subtables belong to the parent table
+// ---------------------------------------------------------------------------
+
+test('stripTomlSection removes dotted subtables of [mcp_servers.graft]', () => {
+  const text = [
+    '[mcp_servers.graft]',
+    'command = "graft"',
+    '',
+    '[mcp_servers.graft.env]',
+    'DO_NOT_TRACK = "1"',
+    '',
+    '[mcp_servers.other]',
+    'command = "x"',
+    '',
+  ].join('\n');
+  const { rest, found } = stripTomlSection(text);
+  assert.equal(found, true);
+  assert.ok(!rest.includes('[mcp_servers.graft]'), 'parent table gone');
+  assert.ok(!rest.includes('[mcp_servers.graft.env]'), 'env subtable gone');
+  assert.ok(!rest.includes('DO_NOT_TRACK'), 'subtable content gone');
+  assert.ok(rest.includes('[mcp_servers.other]'), 'foreign table preserved');
+});
+
+test('stripTomlSection removes deeper dotted subtables', () => {
+  const text = [
+    '[mcp_servers.graft]',
+    'command = "graft"',
+    '',
+    '[mcp_servers.graft.env.extra]',
+    'X = "1"',
+    '',
+  ].join('\n');
+  const { rest } = stripTomlSection(text);
+  assert.ok(!rest.includes('mcp_servers.graft'), 'deep subtable gone');
+});
+
+test('stripTomlSection keeps similarly-named foreign tables', () => {
+  for (const header of ['[mcp_servers.graft2]', '[other.graft]', '[graft]']) {
+    const text = [`[mcp_servers.graft]\ncommand = "graft"\n`, `${header}\ncommand = "y"\n`].join('\n');
+    const { rest, found } = stripTomlSection(text);
+    assert.equal(found, true);
+    assert.ok(!rest.includes('[mcp_servers.graft]\n'), 'own table gone');
+    assert.ok(rest.includes(header), `${header} preserved`);
+  }
+});
+
+test('stripTomlSection leaves files without graft untouched', () => {
+  const text = '[mcp_servers.other]\ncommand = "x"\n';
+  const { rest, found } = stripTomlSection(text);
+  assert.equal(found, false);
+  assert.equal(rest, text, 'byte-identical');
+});
+
+test('stripTomlSection removes non-contiguous family tables', () => {
+  const text = [
+    '[mcp_servers.other]',
+    'command = "x"',
+    '',
+    '[mcp_servers.graft]',
+    'command = "graft"',
+    '',
+    '[mcp_servers.graft2]',
+    'command = "y"',
+    '',
+    '[mcp_servers.graft.env]',
+    'DO_NOT_TRACK = "1"',
+    '',
+    '[mcp_servers.graft.foo.bar]',
+    'Z = "2"',
+    '',
+    '[other.graft]',
+    'command = "z"',
+    '',
+  ].join('\n');
+  const { rest, found } = stripTomlSection(text);
+  assert.equal(found, true);
+  assert.ok(!rest.includes('mcp_servers.graft]'), 'no graft residue');
+  assert.ok(!rest.includes('mcp_servers.graft.'), 'no graft subtable residue');
+  assert.ok(!rest.includes('DO_NOT_TRACK'), 'subtable content gone');
+  assert.ok(rest.includes('[mcp_servers.other]'), 'other preserved');
+  assert.ok(rest.includes('[mcp_servers.graft2]'), 'graft2 preserved');
+  assert.ok(rest.includes('[other.graft]'), 'other.graft preserved');
+});
+
+test('stripTomlSection removes orphan-only subtable without parent', () => {
+  const text = [
+    '[mcp_servers.other]',
+    'command = "x"',
+    '',
+    '[mcp_servers.graft.env]',
+    'DO_NOT_TRACK = "1"',
+    '',
+  ].join('\n');
+  const { rest, found } = stripTomlSection(text);
+  assert.equal(found, true);
+  assert.ok(!rest.includes('mcp_servers.graft'), 'orphan gone');
+  assert.ok(rest.includes('[mcp_servers.other]'), 'foreign preserved');
+});
+
+for (const quote of ['"', "'"]) {
+  const delimiter = quote.repeat(3);
+  test(`stripTomlSection preserves foreign multiline ${quote} strings`, () => {
+    const foreign = [
+      '[mcp_servers.other.env]',
+      `SCRIPT = ${delimiter}`,
+      '[mcp_servers.graft]',
+      '', '', '',
+      '[mcp_servers.graft.env]',
+      'this is string content',
+      delimiter,
+      'KEEP = "2"',
+      '',
+    ].join('\n');
+    const { rest, found } = stripTomlSection('[mcp_servers.graft]\ncommand = "graft"\n' + foreign);
+    assert.equal(found, true);
+    assert.equal(rest, foreign, 'the foreign string and following key survive byte-for-byte');
+    assert.deepEqual(stripTomlSection(foreign), { rest: foreign, found: false },
+      'table-looking string content is not an owned table');
+  });
+
+  test(`stripTomlSection removes owned multiline ${quote} strings completely`, () => {
+    const foreign = '[mcp_servers.other]\ncommand = "other"\n';
+    const text = [
+      '[mcp_servers.graft]',
+      `NOTE = ${delimiter}`,
+      '[mcp_servers.other]',
+      'still owned string content',
+      delimiter,
+      'command = "graft"',
+      foreign,
+    ].join('\n');
+    assert.deepEqual(stripTomlSection(text), { rest: foreign, found: true });
+  });
+
+  for (const closingLength of [4, 5]) {
+    test(`stripTomlSection recognizes a table after ${closingLength} closing ${quote} quotes`, () => {
+      const foreign = `[mcp_servers.other.env]\nNOTE = ${delimiter}content${quote.repeat(closingLength)}\n`;
+      assert.deepEqual(stripTomlSection(foreign + '[mcp_servers.graft]\ncommand = "graft"\n'),
+        { rest: foreign, found: true });
+    });
+  }
+}
+
+test('stripTomlSection ignores delimiters in comments and single-line strings', () => {
+  const foreign = [
+    '[mcp_servers.other.env]',
+    '# """ and \'\'\' are comments',
+    'A = "\'\'\'" # """',
+    'B = \'"""\'',
+    'C = "escaped \\\" quotation"',
+    '',
+  ].join('\n');
+  assert.deepEqual(stripTomlSection(foreign + '[mcp_servers.graft.env]\nX = "1"\n'),
+    { rest: foreign, found: true });
+});
+
+test('stripTomlSection respects escaped multiline basic-string quotes', () => {
+  const foreign = [
+    '[mcp_servers.other.env]',
+    'SCRIPT = """',
+    'an escaped delimiter: \\"""',
+    '[mcp_servers.graft.env]',
+    'still string content',
+    '"""',
+    'KEEP = "2"',
+    '',
+  ].join('\n');
+  assert.deepEqual(stripTomlSection(foreign + '[mcp_servers.graft]\ncommand = "graft"\n'),
+    { rest: foreign, found: true });
+});
+
+test('Codex registration preserves foreign multiline strings across re-registration', () => {
+  const repo = fresh(); const home = fresh();
+  const foreign = '[mcp_servers.other.env]\nSCRIPT = \'\'\'\n[mcp_servers.graft.env]\ntext\n\'\'\'\nKEEP = "2"\n';
+  mkdirSync(join(home, '.codex'), { recursive: true });
+  const path = join(home, '.codex', 'config.toml');
+  writeFileSync(path, '[mcp_servers.graft]\ncommand = "old"\n' + foreign);
+  assert.deepEqual(registerMcpConfigs(repo, ['agents'], { home }).map((w) => w.action), ['updated']);
+  const registered = readFileSync(path, 'utf8');
+  assert.ok(registered.startsWith(foreign), 'foreign configuration survives the writer');
+  assert.deepEqual(registerMcpConfigs(repo, ['agents'], { home }).map((w) => w.action), ['unchanged']);
+  assert.equal(readFileSync(path, 'utf8'), registered);
+});

test/hosts-retract.test.ts

@@ -32,6 +32,16 @@ function byPath(rs: ReturnType<typeof runRetract>): Map<string, string> {
 
 const BLOCK = '<!-- graft:start -->\n## Graft — old text\nstale guidance\n<!-- graft:end -->';
 
+test('uninstall removes both TOML table families without consuming foreign multiline content', () => {
+  const repo = fresh(); const home = fresh();
+  const foreign = '[mcp_servers.other.env]\nSCRIPT = \'\'\'\n[mcp_servers.trail]\n\n\n[mcp_servers.graft.env]\n\'\'\'\nKEEP = "yes"\n';
+  const path = write(home, '.codex/config.toml', '[mcp_servers.trail]\ncommand = "trail"\n'
+    + foreign + '[mcp_servers.graft.env]\nOLD = "remove"\n[mcp_servers.trail.env]\nOLD = "remove"\n');
+  const result = byPath(runRetract(repo, { apply: true, home }));
+  assert.equal(result.get(path), 'removed');
+  assert.equal(readFileSync(path, 'utf8'), foreign);
+});
+
 // --------------------------------------------------------------------------
 // instruction files
 // --------------------------------------------------------------------------
@@ -335,3 +345,40 @@ test('hosts sharing one path produce a single retraction, not one each', () => {
   const rs = runRetract(d, { global: false }).filter((r) => r.path === agents);
   assert.equal(rs.length, 1, `AGENTS.md should be queued once, got ${rs.length}`);
 });
+
+// ---------------------------------------------------------------------------
+// Issue #546: retract removes dotted subtables, not just the parent table
+// ---------------------------------------------------------------------------
+
+test('[mcp_servers.graft.env] orphan does not survive retract', () => {
+  const d = fresh();
+  const toml = write(d, join('.grok', 'config.toml'),
+    '[mcp_servers.graft]\ncommand = "npx"\nargs = ["-y","@nanonets/graft","mcp"]\n\n[mcp_servers.graft.env]\nDO_NOT_TRACK = "1"\n\n[mcp_servers.keepme]\ncommand = "y"\n');
+  runRetract(d, { apply: true, global: false });
+  const text = readFileSync(toml, 'utf8');
+  assert.ok(!text.includes('mcp_servers.graft'), 'no graft residue of any depth');
+  assert.ok(text.includes('[mcp_servers.keepme]'), 'foreign table preserved');
+});
+
+test('orphan-only graft subtable is removed by retract', () => {
+  const d = fresh();
+  const toml = write(d, join('.grok', 'config.toml'),
+    '[mcp_servers.other]\ncommand = "x"\n\n[mcp_servers.graft.env]\nDO_NOT_TRACK = "1"\n');
+  runRetract(d, { apply: true, global: false });
+  const text = readFileSync(toml, 'utf8');
+  assert.ok(!text.includes('mcp_servers.graft'), 'orphan gone via uninstall path');
+  assert.ok(text.includes('[mcp_servers.other]'), 'foreign preserved');
+});
+
+test('Codex retract preserves table-looking text inside a foreign multiline string', () => {
+  const repo = fresh(); const home = fresh();
+  const foreign = '[mcp_servers.other.env]\nSCRIPT = \'\'\'\n[mcp_servers.graft.env]\ntext\n\'\'\'\nKEEP = "2"\n';
+  const toml = write(home, join('.codex', 'config.toml'),
+    '[mcp_servers.graft]\ncommand = "graft"\n' + foreign);
+  const result = byPath(runRetract(repo, { apply: true, home }));
+  assert.equal(result.get(toml), 'removed');
+  assert.equal(readFileSync(toml, 'utf8'), foreign);
+  const again = byPath(runRetract(repo, { apply: true, home }));
+  assert.equal(again.get(toml), 'absent', 'string content does not trigger another removal');
+  assert.equal(readFileSync(toml, 'utf8'), foreign);
+});