fix(mcp): one normalization for the card and the fingerprint

Two more on #55, both mine, both the same root cause: I hardened one function and
left its sibling, then left a comment describing the property I had removed.

1. describeMcpLaunch still threw on a truthy non-array `args`.

   launchFingerprint learned to tolerate it; the card kept doing
   `(s.args || []).map(...)` and threw `TypeError: .map is not a function` — in the
   helper that renders a SECURITY CONSENT CARD. A string `env` did not throw but
   was arguably worse: it enumerated character indices, so the card asked the user
   to trust `0=e 1=v 2=i 3=l`. Rendering nonsense on a consent prompt is worse than
   rendering nothing.

   My "never throws on a malformed entry" test only covered `args: null`, which is
   why it passed while the real malformed shape crashed. Extended, and verified the
   new cases reproduce the exact TypeError against the old code.

2. The comment above launchFingerprint contradicted the implementation.

   It said a malformed args/env "must not fingerprint the same as an absent one".
   The code collapses both to null, and the tests assert the collapse — I corrected
   the test in the previous round and left the comment that motivated it. Now the
   doc states the conservative semantics and why they are safe: normalizeServer
   rejects those shapes long before the gate, so neither is reachable, and the
   command itself always differentiates.

Fixed structurally rather than pointwise: both now read launchMaterial(), one
canonical normalization. This is the third divergence in this file between two
places implementing the same rule, and a shared helper is the only version that
cannot drift again. A test pins that the card reports the fingerprint that will
actually be stored.

61 mcpConfig cases; 23 suites, 0 failures.

Author: ndemianc

extensions/levelcode-ai/mcpConfig.js

@@ -514,20 +514,38 @@ function previewArgs(args) {
  * truncation helper is the wrong tool for an authorization decision; the cost of a real hash here is
  * one call per server per run.
  */
-function launchFingerprint(server) {
+/**
+ * The launch material, normalized ONCE — what would be executed, in canonical form.
+ *
+ * Shared by launchFingerprint and describeMcpLaunch on purpose. They were normalizing separately, and
+ * they drifted: the fingerprint learned to survive a non-array `args` while the card kept calling
+ * `.map` on it and threw. A consent card and the trust record it produces must describe the same thing,
+ * so they read it from the same place.
+ *
+ * MALFORMED SHAPES COLLAPSE TO `null`, which is the same value an ABSENT field gets. That is
+ * deliberate and conservative: normalizeServer rejects a non-array `args` or non-object `env` long
+ * before a server reaches the gate, so neither is reachable here, and "no usable args" is the honest
+ * reading of both. The command itself always differentiates. (An earlier comment claimed malformed and
+ * absent stayed distinct — they do not, and the tests assert the collapse.)
+ *
+ * Env pairs stay STRUCTURAL — [[k, v]] sorted — never joined into "k=v". Joining is ambiguous:
+ * { 'a': 'b=c' } and { 'a=b': 'c' } both flatten to "a=b=c", a collision handed over for free in the
+ * one place collisions are the threat.
+ */
+function launchMaterial(server) {
 	const s = server || {};
-	// Non-array args / non-object env become null rather than [] or {}: a malformed entry must not
-	// fingerprint the same as an absent one, and `.map` on a string would throw in a function whose
-	// whole job is to be safe to call on anything.
-	const args = Array.isArray(s.args) ? s.args.map(String) : null;
-	const rawEnv = (s.env && typeof s.env === 'object' && !Array.isArray(s.env)) ? s.env : null;
-	// Pairs stay STRUCTURAL — [[k, v]] — instead of being joined into "k=v". Joining is ambiguous:
-	// { 'a': 'b=c' } and { 'a=b': 'c' } both flatten to "a=b=c", which is a collision handed over for
-	// free in the one function where collisions are the threat.
-	const env = rawEnv ? Object.keys(rawEnv).sort().map((k) => [k, String(rawEnv[k])]) : null;
-
-	const material = JSON.stringify({ command: String(s.command || ''), args: args, env: env });
-	return crypto.createHash('sha256').update(material, 'utf8').digest('hex');
+	const env = (s.env && typeof s.env === 'object' && !Array.isArray(s.env)) ? s.env : null;
+	return {
+		command: String(s.command || ''),
+		args: Array.isArray(s.args) ? s.args.map(String) : null,
+		env: env ? Object.keys(env).sort().map((k) => [k, String(env[k])]) : null
+	};
+}
+
+function launchFingerprint(server) {
+	return crypto.createHash('sha256')
+		.update(JSON.stringify(launchMaterial(server)), 'utf8')
+		.digest('hex');
 }
 
 /**
@@ -560,14 +578,16 @@ function rememberLaunchTrust(server, store) {
  */
 function describeMcpLaunch(server) {
 	const s = server || {};
-	const quote = (a) => (/[\s"']/.test(String(a)) ? JSON.stringify(String(a)) : String(a));
-	const env = s.env || {};
-	const envLines = Object.keys(env).sort().map((k) => k + '=' + String(env[k]));
+	// Read from launchMaterial, not from `server` directly. Doing its own normalization is what let this
+	// throw on a string `args` (`.map` is not a function) and render `0=e 1=v 2=i 3=l` for a string
+	// `env` — junk on the one card whose whole purpose is showing the user exactly what will run.
+	const material = launchMaterial(s);
+	const quote = (a) => (/[\s"']/.test(a) ? JSON.stringify(a) : a);
 	return {
 		server: String(s.name || ''),
 		origin: String(s.origin || ''),
-		commandLine: [String(s.command || '')].concat((s.args || []).map(quote)).join(' '),
-		envLines: envLines,
+		commandLine: [material.command].concat((material.args || []).map(quote)).join(' ').trim(),
+		envLines: (material.env || []).map(([k, v]) => k + '=' + v),
 		fingerprint: launchFingerprint(s)
 	};
 }

extensions/levelcode-ai/test/mcpConfig.test.js

@@ -762,6 +762,37 @@ test('G1: describeMcpLaunch never throws on a malformed entry', () => {
 	assert.doesNotThrow(() => M.describeMcpLaunch({}));
 	assert.doesNotThrow(() => M.describeMcpLaunch({ name: 'x', args: null, env: null }));
 	assert.strictEqual(M.describeMcpLaunch({}).commandLine, '');
+
+	// The shapes this test USED to miss. It only covered `null`, so a truthy non-array `args` still
+	// reached `.map` and threw — in the helper that renders a security consent card.
+	assert.doesNotThrow(() => M.describeMcpLaunch({ name: 'x', command: 'c', args: 'evil' }));
+	assert.doesNotThrow(() => M.describeMcpLaunch({ name: 'x', command: 'c', args: 42 }));
+	assert.doesNotThrow(() => M.describeMcpLaunch({ name: 'x', command: 'c', env: 'evil' }));
+	assert.doesNotThrow(() => M.describeMcpLaunch({ name: 'x', command: 'c', env: [] }));
+});
+
+test('G1: a malformed entry renders nothing rather than junk on the consent card', () => {
+	// A string env used to enumerate its character indices — the card would ask the user to trust
+	// `0=e 1=v 2=i 3=l`. Showing nonsense on a consent prompt is worse than showing nothing.
+	const strEnv = M.describeMcpLaunch({ name: 'x', command: 'c', env: 'evil' });
+	assert.deepStrictEqual(strEnv.envLines, [], 'no invented env lines');
+	assert.strictEqual(strEnv.commandLine, 'c', 'the command still shows');
+
+	const strArgs = M.describeMcpLaunch({ name: 'x', command: 'c', args: 'evil' });
+	assert.strictEqual(strArgs.commandLine, 'c', 'a malformed args contributes nothing, not "c e v i l"');
+});
+
+test('G1: the card and the fingerprint read the SAME normalized material', () => {
+	// They normalized separately once and drifted — the fingerprint tolerated a non-array args while
+	// the card threw on it. A consent card and the trust it produces must describe one thing.
+	const weird = { name: 'x', command: 'c', args: 'evil', env: 'evil' };
+	assert.strictEqual(
+		M.describeMcpLaunch(weird).fingerprint,
+		M.launchFingerprint(weird),
+		'the card reports the fingerprint that will actually be stored'
+	);
+	// And the card reflects what the fingerprint covers: both ignore the malformed fields.
+	assert.strictEqual(M.launchFingerprint(weird), M.launchFingerprint({ name: 'x', command: 'c' }));
 });
 
 console.log('\nmcpConfig.js: ' + n + ' tests passed.');