From f20c76f4eaf38142742b3abfded004722f72d9d6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E6=9D=A8=E8=89=BA=E9=9F=AC=28yangyitao=29?=
 <yangyitao100@sina.com>
Date: Fri, 13 Mar 2026 13:06:04 +0000
Subject: [PATCH] fix(skills): preserve skill>global precedence when global
 pass runs first in concurrent sessions

Add hasSkillOwner flag to ActiveSkillEnvEntry. When a per-skill acquisition
comes in on a key that was previously only held by global-env passes, upgrade
the stored value and process.env to the skill's value. This ensures the
skill > global precedence invariant holds even when sessions overlap and the
global pass executes before the per-skill pass of a concurrent session.

Also add a concurrent-session regression test covering this scenario.
---
 .../env-overrides.skills-global-env.test.ts   | 43 +++++++++++++++++++
 src/agents/skills/env-overrides.ts            | 22 ++++++++--
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/src/agents/skills/env-overrides.skills-global-env.test.ts b/src/agents/skills/env-overrides.skills-global-env.test.ts
index 750996840c8..9f0ad1889be 100644
--- a/src/agents/skills/env-overrides.skills-global-env.test.ts
+++ b/src/agents/skills/env-overrides.skills-global-env.test.ts
@@ -144,6 +144,49 @@ describe("skills global env", () => {
     expect(process.env["SKILL_API_KEY"]).toBeUndefined();
   });
 
+  it("skill-level env wins over global env when the global pass ran first (concurrent sessions)", () => {
+    // Simulate session A running its global pass before session B runs its per-skill pass.
+    // Session A: only has a global env, no per-skill override for SHARED_KEY.
+    const configA = makeConfig({
+      skills: {
+        env: { SHARED_KEY: "global-value" },
+      },
+    });
+    const revertA = applySkillEnvOverrides({
+      skills: [makeSkillEntry("skill-a")],
+      config: configA,
+    });
+
+    // After session A's passes, SHARED_KEY should be "global-value".
+    expect(process.env["SHARED_KEY"]).toBe("global-value");
+
+    // Session B: has an explicit per-skill override for SHARED_KEY.
+    // Even though the global pass already acquired SHARED_KEY, session B's
+    // skill-level override must take precedence.
+    const configB = makeConfig({
+      skills: {
+        env: { SHARED_KEY: "global-value" },
+        entries: { "skill-b": { env: { SHARED_KEY: "skill-b-value" } } },
+      },
+    });
+    const revertB = applySkillEnvOverrides({
+      skills: [makeSkillEntry("skill-b")],
+      config: configB,
+    });
+
+    expect(process.env["SHARED_KEY"]).toBe("skill-b-value");
+
+    // Reverting B: A still holds the key (refcount > 0), so the key persists.
+    // The stored value reflects the last skill-level upgrade; the important
+    // invariant is that the key is not prematurely deleted.
+    revertB();
+    expect(process.env["SHARED_KEY"]).toBeDefined();
+
+    // Reverting A: no more owners, key should be gone.
+    revertA();
+    expect(process.env["SHARED_KEY"]).toBeUndefined();
+  });
+
   it("applySkillEnvOverridesFromSnapshot injects global env for a skill with no entries config", () => {
     const config = makeConfig({
       skills: {
diff --git a/src/agents/skills/env-overrides.ts b/src/agents/skills/env-overrides.ts
index 621954ff2ee..f6f20ff4e40 100644
--- a/src/agents/skills/env-overrides.ts
+++ b/src/agents/skills/env-overrides.ts
@@ -15,6 +15,13 @@ 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.
+   */
+  hasSkillOwner: boolean;
 };
 
 /**
@@ -30,11 +37,19 @@ export function getActiveSkillEnvKeys(): ReadonlySet<string> {
   return new Set(activeSkillEnvEntries.keys());
 }
 
-function acquireActiveSkillEnvKey(key: string, value: string): boolean {
+function acquireActiveSkillEnvKey(key: string, value: string, isSkillOverride = false): boolean {
   const active = activeSkillEnvEntries.get(key);
   if (active) {
     active.count += 1;
-    if (process.env[key] === undefined) {
+    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;
+    } else if (process.env[key] === undefined) {
       process.env[key] = active.value;
     }
     return true;
@@ -46,6 +61,7 @@ function acquireActiveSkillEnvKey(key: string, value: string): boolean {
     baseline: process.env[key],
     value,
     count: 1,
+    hasSkillOwner: isSkillOverride,
   });
   return true;
 }
@@ -197,7 +213,7 @@ function applySkillConfigEnvOverrides(params: {
   }
 
   for (const [envKey, envValue] of Object.entries(sanitized.allowed)) {
-    if (!acquireActiveSkillEnvKey(envKey, envValue)) {
+    if (!acquireActiveSkillEnvKey(envKey, envValue, /* isSkillOverride */ true)) {
       continue;
     }
     updates.push({ key: envKey });