From 623d683e4090be1d9fdc744aa84255a14aea9d06 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Wed, 22 Jul 2026 12:14:06 -0700
Subject: [PATCH] fix(thin-client): keep ambient scope out of get_skill's
 non-scope source_id param

get_skill's source_id is a mode switch (host catalog vs brain-resident-pack
lookup), not a read-scope filter. Ambient GBRAIN_SOURCE / .gbrain-source
injection would silently reroute 'gbrain skill <name>' on thin clients to
getResidentSkillDetail. Exclude it via NON_SCOPE_SOURCE_ID_OPS; explicit
--source-id still passes through, explicit --source errors with a hint.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 src/cli.ts                            | 14 +++++++++++---
 test/thin-client-source-scope.test.ts | 27 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/cli.ts b/src/cli.ts
index ee5efe251..c798ee195 100755
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -827,6 +827,12 @@ export function parseOpArgs(op: Operation, args: string[]): Record<string, unkno
  * ambient env/dotfile scope with nowhere to send it is ignored, matching the
  * pre-fix behavior for non-scopeable ops. Exported for tests.
  */
+// Ops whose `source_id` wire param is NOT read-scope semantics: get_skill's
+// source_id flips the lookup from host catalog to brain-resident-pack
+// (getResidentSkillDetail). Ambient env/dotfile scope must never leak into
+// these; an explicit --source-id still passes through untouched above.
+const NON_SCOPE_SOURCE_ID_OPS = new Set(['get_skill']);
+
 export function applyThinClientSourceScope(
   op: Operation,
   params: Record<string, unknown>,
@@ -846,11 +852,13 @@ export function applyThinClientSourceScope(
   }
   const resolved = resolveSourceIdEngineFree(explicit, cwd);
   if (!resolved) return;
-  if (!('source_id' in op.params)) {
+  if (!('source_id' in op.params) || NON_SCOPE_SOURCE_ID_OPS.has(op.name)) {
     if (explicit) {
+      const hint = NON_SCOPE_SOURCE_ID_OPS.has(op.name)
+        ? `(its source_id parameter is not a scope filter; pass --source-id explicitly if you mean it)`
+        : `(the remote op has no source_id parameter; the server scopes it to your grant)`;
       throw new Error(
-        `gbrain ${op.cliHints?.name || op.name} does not accept --source on a thin-client install ` +
-        `(the remote op has no source_id parameter; the server scopes it to your grant).`,
+        `gbrain ${op.cliHints?.name || op.name} does not accept --source on a thin-client install ${hint}.`,
       );
     }
     return; // ambient env/dotfile scope with nowhere to send it
diff --git a/test/thin-client-source-scope.test.ts b/test/thin-client-source-scope.test.ts
index e814c6f6a..d366f319d 100644
--- a/test/thin-client-source-scope.test.ts
+++ b/test/thin-client-source-scope.test.ts
@@ -105,6 +105,33 @@ describe('applyThinClientSourceScope (#2098)', () => {
     });
   });
 
+  test('get_skill: ambient scope never leaks into its non-scope source_id param', async () => {
+    await withEnv({ GBRAIN_SOURCE: 'wiki' }, () => {
+      const op = operationsByName.get_skill;
+      expect('source_id' in op.params).toBe(true); // has the param, but it is a mode switch
+      const params: Record<string, unknown> = { name: 'ingest' };
+      applyThinClientSourceScope(op, params, '/');
+      expect(params.source_id).toBeUndefined(); // would flip host catalog → brain-pack lookup
+    });
+  });
+
+  test('get_skill: explicit --source errors instead of masquerading as --source-id', async () => {
+    await withEnv({ GBRAIN_SOURCE: undefined }, () => {
+      const op = operationsByName.get_skill;
+      const params: Record<string, unknown> = { name: 'ingest', source: 'wiki' };
+      expect(() => applyThinClientSourceScope(op, params, '/')).toThrow(/--source-id/);
+    });
+  });
+
+  test('get_skill: explicit --source-id passes through untouched', async () => {
+    await withEnv({ GBRAIN_SOURCE: 'gstack' }, () => {
+      const op = operationsByName.get_skill;
+      const params: Record<string, unknown> = { name: 'ingest', source_id: 'wiki' };
+      applyThinClientSourceScope(op, params, '/');
+      expect(params.source_id).toBe('wiki');
+    });
+  });
+
   test('no scope from any tier leaves params unchanged', async () => {
     await withEnv({ GBRAIN_SOURCE: undefined }, () => {
       const params = parseOpArgs(queryOp, ['find things']);