-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(core): inject context env vars (session/agent/prompt ID) into shell subprocesses #4649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
6982b2f
feat(core): inject context env vars (session/agent/prompt ID) into sh…
yiliang114 478a20e
fix(core): use module-level flag to guard session env claim
yiliang114 a2ab29a
fix(test): use bracket notation for index signature properties
yiliang114 6ec9525
fix(core): guard process.env assignment for mocked process environments
yiliang114 96eb00c
test(config): add coverage for sessionEnvClaimed guard
yiliang114 63a8356
test(config): add startNewSession env var test and clarify comment
yiliang114 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; | ||
|
|
||
| /** | ||
| * Tests for the module-level `sessionEnvClaimed` guard in Config. | ||
| * | ||
| * The guard ensures that only the first Config instance in a process sets | ||
| * `process.env['QWEN_CODE_SESSION_ID']`, preventing throwaway instances | ||
| * (e.g. telemetry-only) from overwriting the real session's ID. | ||
| * | ||
| * We use `vi.isolateModules` to get a fresh module scope (resetting the | ||
| * module-level flag) for each test. | ||
| */ | ||
|
|
||
| // Shared mocks needed by Config constructor | ||
| vi.mock('node:fs'); | ||
| vi.mock('node:fs/promises'); | ||
| vi.mock('../telemetry/index.js', () => ({ | ||
| QwenLogger: vi.fn().mockImplementation(() => ({ | ||
| logStartSessionEvent: vi.fn().mockResolvedValue(undefined), | ||
| logEndSessionEvent: vi.fn().mockResolvedValue(undefined), | ||
| shutdown: vi.fn().mockResolvedValue(undefined), | ||
| })), | ||
| DEFAULT_TELEMETRY_TARGET: 'none', | ||
| DEFAULT_OTLP_ENDPOINT: '', | ||
| isTelemetrySdkInitialized: vi.fn().mockReturnValue(false), | ||
| shutdownTelemetry: vi.fn().mockResolvedValue(undefined), | ||
| refreshSessionContext: vi.fn(), | ||
| })); | ||
| vi.mock('../core/contentGenerator.js', () => ({ | ||
| resolveContentGeneratorConfigWithSources: vi.fn().mockReturnValue({ | ||
| config: { model: 'test-model', apiKey: 'test-key' }, | ||
| sources: {}, | ||
| }), | ||
| createContentGeneratorConfig: vi.fn().mockReturnValue({}), | ||
| createContentGenerator: vi.fn().mockReturnValue({}), | ||
| AuthType: { API_KEY: 'apiKey' }, | ||
| })); | ||
| vi.mock('../core/baseLlmClient.js'); | ||
| vi.mock('../core/toolHookTriggers.js', () => ({ | ||
| fireNotificationHook: vi.fn().mockResolvedValue({}), | ||
| })); | ||
| vi.mock('../services/skillManager.js', () => { | ||
| const SkillManagerMock = vi.fn(); | ||
| SkillManagerMock.prototype.startWatching = vi | ||
| .fn() | ||
| .mockResolvedValue(undefined); | ||
| SkillManagerMock.prototype.refreshCache = vi | ||
| .fn() | ||
| .mockResolvedValue(undefined); | ||
| SkillManagerMock.prototype.stopWatching = vi.fn(); | ||
| SkillManagerMock.prototype.listSkills = vi.fn().mockResolvedValue([]); | ||
| SkillManagerMock.prototype.addChangeListener = vi.fn(); | ||
| SkillManagerMock.prototype.removeChangeListener = vi.fn(); | ||
| SkillManagerMock.prototype.matchAndActivateByPath = vi | ||
| .fn() | ||
| .mockResolvedValue([]); | ||
| SkillManagerMock.prototype.matchAndActivateByPaths = vi | ||
| .fn() | ||
| .mockResolvedValue([]); | ||
| return { SkillManager: SkillManagerMock }; | ||
| }); | ||
| vi.mock('../subagents/subagent-manager.js', () => { | ||
| const SubagentManagerMock = vi.fn(); | ||
| SubagentManagerMock.prototype.loadSessionSubagents = vi.fn(); | ||
| SubagentManagerMock.prototype.addChangeListener = vi | ||
| .fn() | ||
| .mockReturnValue(() => {}); | ||
| SubagentManagerMock.prototype.listSubagents = vi.fn().mockResolvedValue([]); | ||
| return { SubagentManager: SubagentManagerMock }; | ||
| }); | ||
| vi.mock('../ide/ide-client.js', () => ({ | ||
| IdeClient: { | ||
| getInstance: vi.fn().mockResolvedValue({ | ||
| getConnectionStatus: vi.fn(), | ||
| initialize: vi.fn(), | ||
| shutdown: vi.fn(), | ||
| }), | ||
| }, | ||
| })); | ||
| vi.mock('../memory/const.js', () => ({ | ||
| setGeminiMdFilename: vi.fn(), | ||
| })); | ||
|
|
||
| import * as fs from 'node:fs'; | ||
| import type { Mock } from 'vitest'; | ||
| import type { ConfigParameters } from './config.js'; | ||
|
|
||
| const baseParams: ConfigParameters = { | ||
| cwd: '/tmp', | ||
| targetDir: '/tmp', | ||
| debugMode: false, | ||
| model: 'test-model', | ||
| telemetry: { enabled: false }, | ||
| usageStatisticsEnabled: false, | ||
| overrideExtensions: [], | ||
| }; | ||
|
|
||
| describe('Config sessionEnvClaimed guard', () => { | ||
| let originalEnv: string | undefined; | ||
|
|
||
| beforeEach(() => { | ||
| originalEnv = process.env['QWEN_CODE_SESSION_ID']; | ||
| delete process.env['QWEN_CODE_SESSION_ID']; | ||
|
|
||
| (fs.existsSync as Mock).mockReturnValue(true); | ||
| (fs.readdirSync as Mock).mockReturnValue([]); | ||
| (fs.statSync as Mock).mockReturnValue({ | ||
| isDirectory: vi.fn().mockReturnValue(true), | ||
| }); | ||
| vi.mocked(fs.realpathSync).mockImplementation((p) => String(p)); | ||
| (fs.mkdirSync as Mock).mockImplementation(() => undefined); | ||
| (fs.writeFileSync as Mock).mockImplementation(() => undefined); | ||
| (fs.renameSync as Mock).mockImplementation(() => undefined); | ||
| (fs.copyFileSync as Mock).mockImplementation(() => undefined); | ||
| (fs.unlinkSync as Mock).mockImplementation(() => undefined); | ||
| (fs.readFileSync as Mock).mockImplementation(() => undefined); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalEnv !== undefined) { | ||
| process.env['QWEN_CODE_SESSION_ID'] = originalEnv; | ||
| } else { | ||
| delete process.env['QWEN_CODE_SESSION_ID']; | ||
| } | ||
| vi.resetModules(); | ||
| }); | ||
|
|
||
| it('first Config sets process.env QWEN_CODE_SESSION_ID to its sessionId', async () => { | ||
| const { Config } = await import('./config.js'); | ||
| const config = new Config({ ...baseParams }); | ||
|
|
||
| expect(process.env['QWEN_CODE_SESSION_ID']).toBe(config.getSessionId()); | ||
| }); | ||
|
|
||
| it('subsequent Config does not overwrite the env var set by the first', async () => { | ||
| const { Config } = await import('./config.js'); | ||
| const firstConfig = new Config({ ...baseParams }); | ||
| const firstSessionId = firstConfig.getSessionId(); | ||
|
|
||
| // Second Config (e.g. telemetry-only throwaway instance) | ||
| const secondConfig = new Config({ | ||
| ...baseParams, | ||
| sessionId: 'throwaway-session-id', | ||
| }); | ||
|
|
||
| // The env var should still be the first config's session ID | ||
| expect(process.env['QWEN_CODE_SESSION_ID']).toBe(firstSessionId); | ||
| expect(process.env['QWEN_CODE_SESSION_ID']).not.toBe( | ||
| secondConfig.getSessionId(), | ||
| ); | ||
| }); | ||
|
|
||
| it('startNewSession updates env var to the new session ID', async () => { | ||
| const { Config } = await import('./config.js'); | ||
| const config = new Config({ ...baseParams }); | ||
| const originalSessionId = config.getSessionId(); | ||
|
|
||
| expect(process.env['QWEN_CODE_SESSION_ID']).toBe(originalSessionId); | ||
|
|
||
| // Simulate /clear or session switch | ||
| config.startNewSession('new-session-uuid-123'); | ||
|
|
||
| expect(process.env['QWEN_CODE_SESSION_ID']).toBe('new-session-uuid-123'); | ||
| expect(process.env['QWEN_CODE_SESSION_ID']).not.toBe(originalSessionId); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Qwen | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, expect, it, beforeEach, afterEach } from 'vitest'; | ||
| import { getShellContextEnvVars } from './shellContextEnv.js'; | ||
| import { runWithAgentContext } from '../agents/runtime/agent-context.js'; | ||
| import { promptIdContext } from './promptIdContext.js'; | ||
|
|
||
| describe('getShellContextEnvVars', () => { | ||
| let originalSessionId: string | undefined; | ||
|
|
||
| beforeEach(() => { | ||
| originalSessionId = process.env['QWEN_CODE_SESSION_ID']; | ||
| delete process.env['QWEN_CODE_SESSION_ID']; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalSessionId !== undefined) { | ||
| process.env['QWEN_CODE_SESSION_ID'] = originalSessionId; | ||
| } else { | ||
| delete process.env['QWEN_CODE_SESSION_ID']; | ||
| } | ||
| }); | ||
|
|
||
| it('returns empty strings for agent/prompt when no context is available', () => { | ||
| const env = getShellContextEnvVars(); | ||
| expect(env).toEqual({ | ||
| QWEN_CODE_AGENT_ID: '', | ||
| QWEN_CODE_PROMPT_ID: '', | ||
| }); | ||
| }); | ||
|
|
||
| it('returns QWEN_CODE_SESSION_ID when set in process.env', () => { | ||
| process.env['QWEN_CODE_SESSION_ID'] = 'test-session-123'; | ||
| const env = getShellContextEnvVars(); | ||
| expect(env['QWEN_CODE_SESSION_ID']).toBe('test-session-123'); | ||
| }); | ||
|
|
||
| it('returns QWEN_CODE_AGENT_ID when called within agent context', async () => { | ||
| const env = await runWithAgentContext('my-agent-42', async () => | ||
| getShellContextEnvVars(), | ||
| ); | ||
| expect(env['QWEN_CODE_AGENT_ID']).toBe('my-agent-42'); | ||
| }); | ||
|
|
||
| it('returns QWEN_CODE_PROMPT_ID when called within prompt context', () => { | ||
| const env = promptIdContext.run('prompt-abc', () => | ||
| getShellContextEnvVars(), | ||
| ); | ||
| expect(env['QWEN_CODE_PROMPT_ID']).toBe('prompt-abc'); | ||
| }); | ||
|
|
||
| it('returns all vars when all contexts are active', async () => { | ||
| process.env['QWEN_CODE_SESSION_ID'] = 'sess-uuid'; | ||
| const env = await runWithAgentContext('agent-xyz', async () => | ||
| promptIdContext.run('prompt-456', () => getShellContextEnvVars()), | ||
| ); | ||
| expect(env).toEqual({ | ||
| QWEN_CODE_SESSION_ID: 'sess-uuid', | ||
| QWEN_CODE_AGENT_ID: 'agent-xyz', | ||
| QWEN_CODE_PROMPT_ID: 'prompt-456', | ||
| }); | ||
| }); | ||
|
|
||
| it('sets empty string for agent/prompt to override inherited env', () => { | ||
| // Simulates a nested qwen-code process where parent injected these | ||
| const env = getShellContextEnvVars(); | ||
| expect(env['QWEN_CODE_AGENT_ID']).toBe(''); | ||
| expect(env['QWEN_CODE_PROMPT_ID']).toBe(''); | ||
| // Empty strings will overwrite any stale inherited values in process.env | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Qwen | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| /** | ||
| * Returns context environment variables to inject into shell subprocesses. | ||
| * | ||
| * Reads dynamic context (agent ID, prompt ID) from AsyncLocalStorage at | ||
| * call time, and session ID from process.env (set by Config at session | ||
| * start). This enables downstream scripts to identify which session, | ||
| * agent, and prompt triggered their execution — useful for tracing, | ||
| * audit logging, and business context correlation. | ||
| * | ||
| * Must be called at spawn time within the executing async context to | ||
| * capture the correct agent/prompt frame. | ||
| */ | ||
|
|
||
| import { getCurrentAgentId } from '../agents/runtime/agent-context.js'; | ||
| import { promptIdContext } from './promptIdContext.js'; | ||
|
|
||
| export function getShellContextEnvVars(): Record<string, string> { | ||
| const env: Record<string, string> = {}; | ||
|
|
||
| const sessionId = process.env['QWEN_CODE_SESSION_ID']; | ||
| if (sessionId) { | ||
| env['QWEN_CODE_SESSION_ID'] = sessionId; | ||
| } | ||
|
|
||
| // For agent/prompt IDs: explicitly set empty string when no ALS context | ||
| // exists, so that stale values inherited from a parent qwen-code process | ||
| // (via process.env spread) are overwritten rather than leaked. | ||
| const agentId = getCurrentAgentId(); | ||
| env['QWEN_CODE_AGENT_ID'] = agentId ?? ''; | ||
|
yiliang114 marked this conversation as resolved.
|
||
|
|
||
| const promptId = promptIdContext.getStore(); | ||
| env['QWEN_CODE_PROMPT_ID'] = promptId ?? ''; | ||
|
yiliang114 marked this conversation as resolved.
|
||
|
|
||
| return env; | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.