The Codex installer's `findNextTableHeader` skipped `[[array-of-tables]]` headers instead of treating them as a block boundary, so any `[[...]]` block after `[mcp_servers.codegraph]` in ~/.codex/config.toml was silently deleted on install/upgrade/uninstall. Now treats both `[...]` and `[[...]]` as boundaries, with a small line lexer so header-shaped text inside multiline strings/arrays isn't mistaken for a boundary. Adds round-trip regression coverage (install → reinstall → uninstall) + CHANGELOG entry. Fixes #1351. Supersedes #624. Thanks @KtzeAbyss.
This commit is contained in:
Yuc
@@ -48,6 +48,7 @@ Full details in the entries below.
|
||||
|
||||
### Fixes
|
||||
|
||||
- Codex installs, upgrades, and uninstalls now preserve TOML array-of-table sections that appear after CodeGraph's MCP configuration instead of accidentally removing them. (#1351)
|
||||
- TypeScript, TSX, and JavaScript files now parse with up-to-date grammars — modern syntax such as `using` declarations and import attributes no longer trips parse errors that could drop surrounding symbols. (The previously bundled grammars dated from 2023.)
|
||||
- Rust files also parse with an up-to-date grammar now (the previously bundled build dated from 2023), which additionally sharpens method-call attribution: calls through struct fields resolve with receiver context instead of falling back to ambiguous bare-name matching, removing a class of wrong call edges on common names like `len` and `start`.
|
||||
- Ruby files also parse with an up-to-date grammar now (the previously bundled build dated from early 2024), which fixes a misparse of safe-navigation operator-method calls (`recv&.!= x`) that had recorded the wrong callee name.
|
||||
|
||||
@@ -876,6 +876,48 @@ describe('Installer targets — partial-state idempotency', () => {
|
||||
expect(after).not.toContain('enabled = true');
|
||||
});
|
||||
|
||||
it('codex: install, re-install, and uninstall preserve trailing array-of-tables siblings', () => {
|
||||
const codex = getTarget('codex')!;
|
||||
const tomlPath = path.join(tmpHome, '.codex', 'config.toml');
|
||||
fs.mkdirSync(path.dirname(tomlPath), { recursive: true });
|
||||
const historyTables = [
|
||||
'[[history]]',
|
||||
'id = 1',
|
||||
'note = "keep first"',
|
||||
'',
|
||||
'[[history]]',
|
||||
'id = 2',
|
||||
'note = "keep second"',
|
||||
'',
|
||||
].join('\n');
|
||||
fs.writeFileSync(tomlPath, [
|
||||
'[mcp_servers.codegraph]',
|
||||
'command = "old-codegraph"',
|
||||
'args = ["old"]',
|
||||
'description = """',
|
||||
'header-shaped text inside a multiline string:',
|
||||
'[[not-a-table]]',
|
||||
'still part of the string',
|
||||
'"""',
|
||||
'',
|
||||
historyTables,
|
||||
].join('\n'));
|
||||
|
||||
const first = codex.install('global', { autoAllow: false });
|
||||
expect(first.files.find((f) => f.path === tomlPath)?.action).toBe('updated');
|
||||
const afterInstall = fs.readFileSync(tomlPath, 'utf-8');
|
||||
expect(afterInstall).toContain('command = "codegraph"');
|
||||
expect(afterInstall).not.toContain('[[not-a-table]]');
|
||||
expect(afterInstall.endsWith(historyTables)).toBe(true);
|
||||
|
||||
const second = codex.install('global', { autoAllow: false });
|
||||
expect(second.files.find((f) => f.path === tomlPath)?.action).toBe('unchanged');
|
||||
expect(fs.readFileSync(tomlPath, 'utf-8')).toBe(afterInstall);
|
||||
|
||||
codex.uninstall('global');
|
||||
expect(fs.readFileSync(tomlPath, 'utf-8')).toBe(historyTables);
|
||||
});
|
||||
|
||||
it('claude: local install writes ./.mcp.json (project scope), not ./.claude.json', () => {
|
||||
const claude = getTarget('claude')!;
|
||||
const result = claude.install('local', { autoAllow: false });
|
||||
@@ -1292,6 +1334,113 @@ describe('Installer targets — TOML serializer (Codex backbone)', () => {
|
||||
expect(content.match(/\[\[foo\]\]/g)?.length).toBe(2);
|
||||
expect(content).toContain('[mcp_servers.codegraph]');
|
||||
});
|
||||
|
||||
it('upsert replaces the managed table without consuming trailing array-of-tables siblings', () => {
|
||||
const historyTables = [
|
||||
'[[history]]',
|
||||
'id = 1',
|
||||
'note = "keep first"',
|
||||
'',
|
||||
'[[history]]',
|
||||
'id = 2',
|
||||
'note = "keep second"',
|
||||
'',
|
||||
].join('\n');
|
||||
const existing = [
|
||||
'[mcp_servers.codegraph]',
|
||||
'command = "old-codegraph"',
|
||||
'args = ["old"]',
|
||||
'',
|
||||
historyTables,
|
||||
].join('\n');
|
||||
const block = buildTomlTable('mcp_servers.codegraph', {
|
||||
command: 'codegraph',
|
||||
args: ['serve', '--mcp'],
|
||||
});
|
||||
|
||||
const { content, action } = upsertTomlTable(existing, 'mcp_servers.codegraph', block);
|
||||
|
||||
expect(action).toBe('replaced');
|
||||
expect(content).toBe(`${block}\n\n${historyTables}`);
|
||||
});
|
||||
|
||||
it('remove preserves trailing array-of-tables siblings byte-for-byte', () => {
|
||||
const historyTables = [
|
||||
'[[history]]',
|
||||
'id = 1',
|
||||
'note = "keep first"',
|
||||
'',
|
||||
'[[history]]',
|
||||
'id = 2',
|
||||
'note = "keep second"',
|
||||
'',
|
||||
].join('\n');
|
||||
const existing = [
|
||||
'[mcp_servers.codegraph]',
|
||||
'command = "codegraph"',
|
||||
'args = ["serve", "--mcp"]',
|
||||
'',
|
||||
historyTables,
|
||||
].join('\n');
|
||||
|
||||
const { content, action } = removeTomlTable(existing, 'mcp_servers.codegraph');
|
||||
|
||||
expect(action).toBe('removed');
|
||||
expect(content).toBe(historyTables);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['table', '[ mcp_servers.other ]'],
|
||||
['array-of-tables', '[[ history ]]'],
|
||||
])('preserves a trailing %s header with inner whitespace', (_kind, siblingHeader) => {
|
||||
const siblingTable = `${siblingHeader}\nvalue = "keep"\n`;
|
||||
const existing = [
|
||||
'[mcp_servers.codegraph]',
|
||||
'command = "old-codegraph"',
|
||||
'args = ["old"]',
|
||||
'',
|
||||
siblingTable,
|
||||
].join('\n');
|
||||
const block = buildTomlTable('mcp_servers.codegraph', {
|
||||
command: 'codegraph',
|
||||
args: ['serve', '--mcp'],
|
||||
});
|
||||
|
||||
const upserted = upsertTomlTable(existing, 'mcp_servers.codegraph', block);
|
||||
const removed = removeTomlTable(existing, 'mcp_servers.codegraph');
|
||||
|
||||
expect(upserted.content).toBe(`${block}\n\n${siblingTable}`);
|
||||
expect(removed.content).toBe(siblingTable);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['basic', '"""'],
|
||||
['literal', "'''"],
|
||||
])('ignores header-shaped text inside a multiline %s string', (_kind, delimiter) => {
|
||||
const historyTable = '[[history]]\nid = 1\n';
|
||||
const existing = [
|
||||
'[mcp_servers.codegraph]',
|
||||
'command = "old-codegraph"',
|
||||
'args = [',
|
||||
` ${delimiter}first line`,
|
||||
'[[not-a-table]]',
|
||||
`last line${delimiter},`,
|
||||
' "serve",',
|
||||
']',
|
||||
'',
|
||||
historyTable,
|
||||
].join('\n');
|
||||
const block = buildTomlTable('mcp_servers.codegraph', {
|
||||
command: 'codegraph',
|
||||
args: ['serve', '--mcp'],
|
||||
});
|
||||
|
||||
const upserted = upsertTomlTable(existing, 'mcp_servers.codegraph', block);
|
||||
const removed = removeTomlTable(existing, 'mcp_servers.codegraph');
|
||||
|
||||
expect(upserted.content).toBe(`${block}\n\n${historyTable}`);
|
||||
expect(removed.content).toBe(historyTable);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Installer — uninstallTargets sweep (codegraph uninstall)', () => {
|
||||
|
||||
+119
-19
@@ -7,13 +7,14 @@
|
||||
*
|
||||
* Strategy: treat the file as text. Find the `[mcp_servers.codegraph]`
|
||||
* header line, splice it (and the lines that follow it until the next
|
||||
* `[...]` header or EOF) in or out. Everything outside that block is
|
||||
* preserved verbatim, byte-for-byte.
|
||||
* `[...]` / `[[...]]` header or EOF) in or out. A small lexical scan keeps
|
||||
* header-shaped text inside multiline values out of the boundary search.
|
||||
* Everything outside that block is preserved verbatim, byte-for-byte.
|
||||
*
|
||||
* Limitations (acceptable for our narrow use):
|
||||
* - Only handles top-level table headers; not array-of-tables or
|
||||
* subtables nested inside `[mcp_servers]` itself (we always write
|
||||
* the full dotted key `[mcp_servers.codegraph]`).
|
||||
* - Only writes a top-level table header. Array-of-tables and sibling
|
||||
* subtables are preserved as opaque blocks (we always write the full
|
||||
* dotted key `[mcp_servers.codegraph]`).
|
||||
* - Doesn't validate sibling TOML — if the file is malformed
|
||||
* elsewhere, our injection won't fix it but won't make it worse.
|
||||
* - Quotes string values with double quotes; escapes `\` and `"`.
|
||||
@@ -79,7 +80,7 @@ export function upsertTomlTable(
|
||||
};
|
||||
}
|
||||
|
||||
// Find the end of this block: next `[...]` header (at line start) or EOF.
|
||||
// Find the end of this block: next table header or EOF.
|
||||
const blockEnd = findNextTableHeader(fileContent, headerIdx + headerLine.length);
|
||||
const existingBlock = fileContent.substring(headerIdx, blockEnd).replace(/\n+$/, '');
|
||||
|
||||
@@ -133,22 +134,121 @@ function findHeaderIndex(content: string, headerLine: string): number {
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the byte index of the next top-level `[...]` table header
|
||||
* (excluding array-of-tables `[[...]]`) starting from `from`, or
|
||||
* return content length when none.
|
||||
* Find the byte index of the next `[...]` or `[[...]]` table header
|
||||
* starting from `from`, or return content length when none.
|
||||
*/
|
||||
function findNextTableHeader(content: string, from: number): number {
|
||||
// Look for "\n[" but skip "\n[[" (array of tables).
|
||||
let i = from;
|
||||
while (i < content.length) {
|
||||
const nlIdx = content.indexOf('\n[', i);
|
||||
if (nlIdx === -1) return content.length;
|
||||
if (content[nlIdx + 2] === '[') {
|
||||
// [[...]] — keep searching past it.
|
||||
i = nlIdx + 2;
|
||||
continue;
|
||||
const state: TomlLexState = { multilineString: null, arrayDepth: 0, inlineTableDepth: 0 };
|
||||
let lineStart = from;
|
||||
let isHeaderRemainder = true;
|
||||
|
||||
while (lineStart < content.length) {
|
||||
const newlineIdx = content.indexOf('\n', lineStart);
|
||||
const lineEnd = newlineIdx === -1 ? content.length : newlineIdx;
|
||||
const line = content.slice(lineStart, lineEnd);
|
||||
|
||||
if (
|
||||
!isHeaderRemainder &&
|
||||
state.multilineString === null &&
|
||||
state.arrayDepth === 0 &&
|
||||
state.inlineTableDepth === 0 &&
|
||||
isTomlTableHeader(line)
|
||||
) {
|
||||
return lineStart;
|
||||
}
|
||||
return nlIdx + 1;
|
||||
|
||||
scanTomlLine(line, state);
|
||||
if (newlineIdx === -1) break;
|
||||
lineStart = newlineIdx + 1;
|
||||
isHeaderRemainder = false;
|
||||
}
|
||||
|
||||
return content.length;
|
||||
}
|
||||
|
||||
type MultilineStringDelimiter = '"""' | "'''";
|
||||
|
||||
interface TomlLexState {
|
||||
multilineString: MultilineStringDelimiter | null;
|
||||
arrayDepth: number;
|
||||
inlineTableDepth: number;
|
||||
}
|
||||
|
||||
const TOML_KEY_PART = String.raw`(?:[A-Za-z0-9_-]+|"(?:\\.|[^"\\])*"|'[^']*')`;
|
||||
const TOML_DOTTED_KEY = String.raw`${TOML_KEY_PART}(?:[ \t]*\.[ \t]*${TOML_KEY_PART})*`;
|
||||
const TOML_TABLE = String.raw`\[[ \t]*${TOML_DOTTED_KEY}[ \t]*\]`;
|
||||
const TOML_ARRAY_TABLE = String.raw`\[\[[ \t]*${TOML_DOTTED_KEY}[ \t]*\]\]`;
|
||||
const TOML_TABLE_HEADER = new RegExp(
|
||||
String.raw`^[ \t]*(?:${TOML_TABLE}|${TOML_ARRAY_TABLE})[ \t]*(?:#.*)?\r?$`
|
||||
);
|
||||
|
||||
function isTomlTableHeader(line: string): boolean {
|
||||
return TOML_TABLE_HEADER.test(line);
|
||||
}
|
||||
|
||||
/** Track value constructs that may legally span lines so bracket-shaped string
|
||||
* content and nested arrays cannot be mistaken for sibling table headers. */
|
||||
function scanTomlLine(line: string, state: TomlLexState): void {
|
||||
for (let i = 0; i < line.length;) {
|
||||
if (state.multilineString !== null) {
|
||||
const end = findMultilineStringEnd(line, i, state.multilineString);
|
||||
if (end === -1) return;
|
||||
i = end + state.multilineString.length;
|
||||
state.multilineString = null;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (line[i] === '#') return;
|
||||
|
||||
const multiline = line.startsWith('"""', i)
|
||||
? '"""'
|
||||
: line.startsWith("'''", i)
|
||||
? "'''"
|
||||
: null;
|
||||
if (multiline !== null) {
|
||||
state.multilineString = multiline;
|
||||
i += multiline.length;
|
||||
continue;
|
||||
}
|
||||
|
||||
const ch = line[i]!;
|
||||
if (ch === '"' || ch === "'") {
|
||||
i = skipSingleLineString(line, i, ch);
|
||||
continue;
|
||||
}
|
||||
if (ch === '[') state.arrayDepth++;
|
||||
else if (ch === ']' && state.arrayDepth > 0) state.arrayDepth--;
|
||||
else if (ch === '{') state.inlineTableDepth++;
|
||||
else if (ch === '}' && state.inlineTableDepth > 0) state.inlineTableDepth--;
|
||||
i++;
|
||||
}
|
||||
}
|
||||
|
||||
function findMultilineStringEnd(
|
||||
line: string,
|
||||
from: number,
|
||||
delimiter: MultilineStringDelimiter,
|
||||
): number {
|
||||
let end = line.indexOf(delimiter, from);
|
||||
while (delimiter === '"""' && end !== -1 && isBackslashEscaped(line, end)) {
|
||||
end = line.indexOf(delimiter, end + 1);
|
||||
}
|
||||
return end;
|
||||
}
|
||||
|
||||
function isBackslashEscaped(line: string, index: number): boolean {
|
||||
let backslashes = 0;
|
||||
for (let i = index - 1; i >= 0 && line[i] === '\\'; i--) backslashes++;
|
||||
return backslashes % 2 === 1;
|
||||
}
|
||||
|
||||
function skipSingleLineString(line: string, start: number, quote: '"' | "'"): number {
|
||||
for (let i = start + 1; i < line.length; i++) {
|
||||
if (quote === '"' && line[i] === '\\') {
|
||||
i++;
|
||||
continue;
|
||||
}
|
||||
if (line[i] === quote) return i + 1;
|
||||
}
|
||||
return line.length;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user