From 051eb7b8d01b163d55558de9f2b660c0adb7b9cc Mon Sep 17 00:00:00 2001
From: yangyitao <17801025312@sina.cn>
Date: Fri, 13 Mar 2026 15:14:02 +0000
Subject: [PATCH] fix(skills): restore global env value when last skill
 override releases
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When session A holds SHARED_KEY via skills.env (global) and session B
temporarily overrides it with a per-skill value, reverting B previously
left process.env reflecting the skill value instead of restoring to the
global value — causing A to run under the wrong credential for the rest
of its lifetime.

Fix: track skillOwnerCount and globalValue on ActiveSkillEnvEntry.
When the last per-skill session releases a key that still has global
owners, downgrade active.value to the saved globalValue and update
process.env immediately. releaseActiveSkillEnvKey and createEnvReverter
now carry an isSkillOverride flag to drive this logic correctly.
---
 src/agents/skills/env-overrides.ts | 56 ++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/src/agents/skills/env-overrides.ts b/src/agents/skills/env-overrides.ts
index f6f20ff4e40..64625aa6fc9 100644
--- a/src/agents/skills/env-overrides.ts
+++ b/src/agents/skills/env-overrides.ts
@@ -9,19 +9,25 @@ import type { SkillEntry, SkillSnapshot } from "./types.js";
 
 const log = createSubsystemLogger("env-overrides");
 
-type EnvUpdate = { key: string };
+type EnvUpdate = { key: string; isSkillOverride?: boolean };
 type SkillConfig = NonNullable<ReturnType<typeof resolveSkillConfig>>;
 type ActiveSkillEnvEntry = {
   baseline: string | undefined;
   value: string;
   count: number;
   /**
-   * True if at least one per-skill (non-global) session currently holds this
-   * key. Used to enforce skill > global precedence across concurrent sessions:
-   * if a global pass acquired the key first, a later per-skill pass must still
-   * win and upgrade the stored/active value.
+   * Number of per-skill (non-global) sessions currently holding this key.
+   * Used to enforce skill > global precedence across concurrent sessions and
+   * to correctly downgrade to the global value when the last skill owner exits.
    */
-  hasSkillOwner: boolean;
+  skillOwnerCount: number;
+  /**
+   * The global-env value saved when the first per-skill session upgraded this
+   * key. Restored to `value` (and `process.env`) when the last skill owner
+   * releases so that any remaining global-env sessions continue to see the
+   * correct credential.
+   */
+  globalValue?: string;
 };
 
 /**
@@ -41,14 +47,16 @@ function acquireActiveSkillEnvKey(key: string, value: string, isSkillOverride =
   const active = activeSkillEnvEntries.get(key);
   if (active) {
     active.count += 1;
-    if (isSkillOverride && !active.hasSkillOwner) {
-      // A per-skill acquisition is taking over a key that was previously only
-      // held by global-env passes. Upgrade to the skill value so that
-      // skill > global precedence is preserved even for concurrent sessions
-      // where the global pass ran first.
-      active.value = value;
-      active.hasSkillOwner = true;
-      process.env[key] = value;
+    if (isSkillOverride) {
+      if (active.skillOwnerCount === 0) {
+        // First per-skill session taking over a key previously held only by
+        // global-env passes. Save the global value so we can restore it when
+        // the last skill owner exits, then upgrade to the skill value.
+        active.globalValue = active.value;
+        active.value = value;
+        process.env[key] = value;
+      }
+      active.skillOwnerCount += 1;
     } else if (process.env[key] === undefined) {
       process.env[key] = active.value;
     }
@@ -61,17 +69,29 @@ function acquireActiveSkillEnvKey(key: string, value: string, isSkillOverride =
     baseline: process.env[key],
     value,
     count: 1,
-    hasSkillOwner: isSkillOverride,
+    skillOwnerCount: isSkillOverride ? 1 : 0,
+    globalValue: undefined,
   });
   return true;
 }
 
-function releaseActiveSkillEnvKey(key: string) {
+function releaseActiveSkillEnvKey(key: string, isSkillOverride = false) {
   const active = activeSkillEnvEntries.get(key);
   if (!active) {
     return;
   }
   active.count -= 1;
+  if (isSkillOverride && active.skillOwnerCount > 0) {
+    active.skillOwnerCount -= 1;
+    if (active.skillOwnerCount === 0 && active.count > 0 && active.globalValue !== undefined) {
+      // The last per-skill owner has released the key, but global-env sessions
+      // still hold a reference. Downgrade to the saved global value so those
+      // sessions see the correct credential for the rest of their lifetime.
+      active.value = active.globalValue;
+      active.globalValue = undefined;
+      process.env[key] = active.value;
+    }
+  }
   if (active.count > 0) {
     if (process.env[key] === undefined) {
       process.env[key] = active.value;
@@ -216,7 +236,7 @@ function applySkillConfigEnvOverrides(params: {
     if (!acquireActiveSkillEnvKey(envKey, envValue, /* isSkillOverride */ true)) {
       continue;
     }
-    updates.push({ key: envKey });
+    updates.push({ key: envKey, isSkillOverride: true });
     process.env[envKey] = activeSkillEnvEntries.get(envKey)?.value ?? envValue;
   }
 }
@@ -224,7 +244,7 @@ function applySkillConfigEnvOverrides(params: {
 function createEnvReverter(updates: EnvUpdate[]) {
   return () => {
     for (const update of updates) {
-      releaseActiveSkillEnvKey(update.key);
+      releaseActiveSkillEnvKey(update.key, update.isSkillOverride ?? false);
     }
   };
 }