From 5f475fc7908b15d192ace4551a62635f7651a2dc Mon Sep 17 00:00:00 2001 From: MachineGunLIn <805435831@qq.com> Date: Sun, 4 Oct 2026 13:18:21 +0800 Subject: [PATCH 1/4] fix(mcp): strip dotted subtables when removing graft TOML section stripTomlSection stopped at the next '[' header, orphaning [mcp_servers.graft.*] subtables (e.g. .env) and breaking Codex CLI config loads after uninstall. Dotted children of the graft table are now consumed with it; foreign tables are untouched. Fixes #546 --- src/hosts/mcp-config.ts | 27 ++++++++++++++--- test/hosts-mcp-config.test.ts | 56 ++++++++++++++++++++++++++++++++++- test/hosts-retract.test.ts | 14 +++++++++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/src/hosts/mcp-config.ts b/src/hosts/mcp-config.ts index 111da50f3..8783dbd28 100644 --- a/src/hosts/mcp-config.ts +++ b/src/hosts/mcp-config.ts @@ -98,16 +98,35 @@ const TOML_HEADER = '[mcp_servers.graft]'; * Remove the `[mcp_servers.graft]` table 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 - * `retract.ts` can never disagree about what "graft's section" means. + * whole file. The table runs from its header through its dotted subtables + * (`[mcp_servers.graft.*]`, which TOML scopes under the parent) to the next + * unrelated `[`-header or EOF, which is exactly the shape {@link upsertCodexToml} + * appends. Exported so the writer and `retract.ts` can never disagree about + * what "graft's section" means. */ export function stripTomlSection(text: string): { rest: string; found: boolean } { const lines = text.split('\n'); const start = lines.findIndex((l) => l.trim() === TOML_HEADER); if (start === -1) return { rest: text, found: false }; + // A dotted subtable (`[mcp_servers.graft.env]`) belongs to the parent table + // and must go with it; anything else starting a new table ends the section. + // The trailing dot keeps similarly-named tables (`[mcp_servers.graft2]`, + // `[other.graft]`) out of the family. + const familyPrefix = `${TOML_HEADER.slice(0, -1)}.`; let end = start + 1; - while (end < lines.length && !lines[end].trimStart().startsWith('[')) end++; + while (end < lines.length) { + const trimmed = lines[end].trimStart(); + if (!trimmed.startsWith('[')) { + end++; + continue; + } + const header = trimmed.split(/\s+/)[0]; + if (header === TOML_HEADER || header.startsWith(familyPrefix)) { + end++; + continue; + } + break; + } const rest = [...lines.slice(0, start), ...lines.slice(end)] .join('\n') .replace(/\n{3,}/g, '\n\n') diff --git a/test/hosts-mcp-config.test.ts b/test/hosts-mcp-config.test.ts index f65966702..7e7b14595 100644 --- a/test/hosts-mcp-config.test.ts +++ b/test/hosts-mcp-config.test.ts @@ -7,7 +7,7 @@ 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-')); } @@ -110,3 +110,57 @@ 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'); +}); diff --git a/test/hosts-retract.test.ts b/test/hosts-retract.test.ts index dee31b8e9..e5cb7d389 100644 --- a/test/hosts-retract.test.ts +++ b/test/hosts-retract.test.ts @@ -335,3 +335,17 @@ 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'); +}); From be9b226483831ef6cc9f71186e1d1ef7b32e0046 Mon Sep 17 00:00:00 2001 From: MachineGunLIn <805435831@qq.com> Date: Sun, 4 Oct 2026 14:42:37 +0800 Subject: [PATCH 2/4] fix(mcp): remove all graft TOML tables including orphans Extend stripTomlSection to whole-file family removal: every [mcp_servers.graft] and [mcp_servers.graft.*] table is dropped whether contiguous or not, including orphaned subtables without a parent. Foreign tables are preserved. --- src/hosts/mcp-config.ts | 44 ++++++++++++++++----------------- test/hosts-mcp-config.test.ts | 46 +++++++++++++++++++++++++++++++++++ test/hosts-retract.test.ts | 10 ++++++++ 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/src/hosts/mcp-config.ts b/src/hosts/mcp-config.ts index 8783dbd28..893b998cc 100644 --- a/src/hosts/mcp-config.ts +++ b/src/hosts/mcp-config.ts @@ -95,39 +95,39 @@ export function mergeJsonKey(id: string, path: string, topKey: string, entry: ob const TOML_HEADER = '[mcp_servers.graft]'; /** - * Remove the `[mcp_servers.graft]` table from a TOML config, returning the rest. + * Remove every canonical graft-owned table 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 through its dotted subtables - * (`[mcp_servers.graft.*]`, which TOML scopes under the parent) to the next - * unrelated `[`-header or EOF, which is exactly the shape {@link upsertCodexToml} - * appends. Exported so the writer and `retract.ts` can never disagree about - * what "graft's section" means. + * whole file. A graft-owned table is `[mcp_servers.graft]` itself or any dotted + * subtable below it (`[mcp_servers.graft.*]`, 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): { rest: string; found: boolean } { const lines = text.split('\n'); - const start = lines.findIndex((l) => l.trim() === TOML_HEADER); - if (start === -1) return { rest: text, found: false }; - // A dotted subtable (`[mcp_servers.graft.env]`) belongs to the parent table - // and must go with it; anything else starting a new table ends the section. // The trailing dot keeps similarly-named tables (`[mcp_servers.graft2]`, // `[other.graft]`) out of the family. const familyPrefix = `${TOML_HEADER.slice(0, -1)}.`; - let end = start + 1; - while (end < lines.length) { - const trimmed = lines[end].trimStart(); - if (!trimmed.startsWith('[')) { - end++; - continue; - } + const isFamilyHeader = (line: string): boolean => { + const trimmed = line.trimStart(); + if (!trimmed.startsWith('[')) return false; const header = trimmed.split(/\s+/)[0]; - if (header === TOML_HEADER || header.startsWith(familyPrefix)) { - end++; - continue; + return header === TOML_HEADER || header.startsWith(familyPrefix); + }; + if (!lines.some(isFamilyHeader)) return { rest: text, found: false }; + let inGraft = false; + const kept: string[] = []; + for (const line of lines) { + if (line.trimStart().startsWith('[')) { + inGraft = isFamilyHeader(line); + if (inGraft) continue; } - break; + if (!inGraft) kept.push(line); } - const rest = [...lines.slice(0, start), ...lines.slice(end)] + const rest = kept .join('\n') .replace(/\n{3,}/g, '\n\n') .replace(/^\n+/, ''); diff --git a/test/hosts-mcp-config.test.ts b/test/hosts-mcp-config.test.ts index 7e7b14595..b767ff493 100644 --- a/test/hosts-mcp-config.test.ts +++ b/test/hosts-mcp-config.test.ts @@ -164,3 +164,49 @@ test('stripTomlSection leaves files without graft untouched', () => { 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'); +}); diff --git a/test/hosts-retract.test.ts b/test/hosts-retract.test.ts index e5cb7d389..9e4a6b136 100644 --- a/test/hosts-retract.test.ts +++ b/test/hosts-retract.test.ts @@ -349,3 +349,13 @@ test('[mcp_servers.graft.env] orphan does not survive retract', () => { 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'); +}); From e36ba64b8bdb7daa49fc7bd897fdcb770dc0a42c Mon Sep 17 00:00:00 2001 From: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:06:54 +0800 Subject: [PATCH 3/4] fix(mcp): preserve multiline strings during Graft removal --- src/hosts/mcp-config.ts | 50 ++++++++++++++++++--- test/hosts-mcp-config.test.ts | 85 +++++++++++++++++++++++++++++++++++ test/hosts-retract.test.ts | 13 ++++++ 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/src/hosts/mcp-config.ts b/src/hosts/mcp-config.ts index 893b998cc..f202f424f 100644 --- a/src/hosts/mcp-config.ts +++ b/src/hosts/mcp-config.ts @@ -94,6 +94,41 @@ export function mergeJsonKey(id: string, path: string, topKey: string, entry: ob /** The `[mcp_servers.graft]` table header, as written and as matched. */ const TOML_HEADER = '[mcp_servers.graft]'; +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 every canonical graft-owned table from a TOML config, returning the rest. * @@ -107,7 +142,7 @@ const TOML_HEADER = '[mcp_servers.graft]'; * `retract.ts` can never disagree about what "graft's section" means. */ export function stripTomlSection(text: string): { rest: string; found: boolean } { - const lines = text.split('\n'); + const lines = text.split(/(?<=\n)/); // The trailing dot keeps similarly-named tables (`[mcp_servers.graft2]`, // `[other.graft]`) out of the family. const familyPrefix = `${TOML_HEADER.slice(0, -1)}.`; @@ -117,19 +152,22 @@ export function stripTomlSection(text: string): { rest: string; found: boolean } const header = trimmed.split(/\s+/)[0]; return header === TOML_HEADER || header.startsWith(familyPrefix); }; - if (!lines.some(isFamilyHeader)) return { rest: text, found: false }; + let multiline: TomlStringQuote | null = null; + let found = false; let inGraft = false; const kept: string[] = []; for (const line of lines) { - if (line.trimStart().startsWith('[')) { + if (multiline === null && line.trimStart().startsWith('[')) { inGraft = isFamilyHeader(line); - if (inGraft) continue; + found ||= inGraft; } if (!inGraft) 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('\n') - .replace(/\n{3,}/g, '\n\n') + .join('') .replace(/^\n+/, ''); return { rest, found: true }; } diff --git a/test/hosts-mcp-config.test.ts b/test/hosts-mcp-config.test.ts index b767ff493..85a627874 100644 --- a/test/hosts-mcp-config.test.ts +++ b/test/hosts-mcp-config.test.ts @@ -210,3 +210,88 @@ test('stripTomlSection removes orphan-only subtable without parent', () => { 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); +}); diff --git a/test/hosts-retract.test.ts b/test/hosts-retract.test.ts index 9e4a6b136..582ca8922 100644 --- a/test/hosts-retract.test.ts +++ b/test/hosts-retract.test.ts @@ -359,3 +359,16 @@ test('orphan-only graft subtable is removed by retract', () => { 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); +}); From 9e8878a5af3baf383057362621fc96814d17c5ef Mon Sep 17 00:00:00 2001 From: Frankie-Xu <92643488+Frankie-Xu@users.noreply.github.com> Date: Wed, 7 Oct 2026 19:03:24 +0800 Subject: [PATCH 4/4] ci: keep CodeQL initialization and analysis versions aligned --- .github/workflows/codeql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 7760095b2..888512d27 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -26,7 +26,7 @@ jobs: persist-credentials: false - name: Initialize CodeQL - uses: github/codeql-action/init@db488ddef3bf6cb639b32c2e9a7c0a7ea8271d28 # v4.37.8 + uses: github/codeql-action/init@1c5b675653bb5c22dbe9b12b556ec555138e09fd # v4.38.1 with: languages: javascript-typescript