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.
This commit is contained in:
parent
1ab8796ccc
commit
f20c76f4ea
@ -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: {
|
||||
|
||||
@ -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 });
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user