-
Notifications
You must be signed in to change notification settings - Fork 3k
feat(cli): reload project runtime after /cd #10263
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
7580e80
a1f53cc
996b257
d223ed6
784a6d2
afbf358
e8d2b12
6d1bdd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -197,6 +197,7 @@ import * as fs from 'node:fs/promises'; | |||||
| import * as os from 'node:os'; | ||||||
| import * as path from 'node:path'; | ||||||
| import type { LoadedSettings } from '../config/settings.js'; | ||||||
| import { formatCronRelocationNotice } from '../config/cron-relocation-notice.js'; | ||||||
| import { | ||||||
| loadSettings, | ||||||
| reloadEnvironment, | ||||||
|
|
@@ -2304,8 +2305,9 @@ function readScopeSettings( | |||||
| async function resolvePreferredMemoryFile( | ||||||
| dir: string, | ||||||
| fallbackFilename: string, | ||||||
| contextFileNames: readonly string[], | ||||||
| ): Promise<string> { | ||||||
| for (const filename of getAllMemoryFilenames()) { | ||||||
| for (const filename of contextFileNames) { | ||||||
| const filePath = path.join(dir, filename); | ||||||
| try { | ||||||
| await fs.access(filePath); | ||||||
|
|
@@ -2321,15 +2323,25 @@ async function resolvePreferredMemoryFile( | |||||
| async function resolveQwenMemoryPaths(params: { | ||||||
| cwd: string; | ||||||
| projectRoot: string; | ||||||
| /** | ||||||
| * The session's context-file names. `/cd` makes these session-scoped | ||||||
| * and never updates the process-global list, so a host asking for the | ||||||
| * memory paths after a move must be answered from the session, or it | ||||||
| * is handed `QWEN.md` for a project whose file is `CONTEXT.md`. | ||||||
| */ | ||||||
| contextFileNames?: readonly string[]; | ||||||
| }): Promise<QwenMemoryPaths> { | ||||||
| const fallbackFilename = getAllMemoryFilenames()[0] ?? 'QWEN.md'; | ||||||
| const contextFileNames = params.contextFileNames ?? getAllMemoryFilenames(); | ||||||
| const fallbackFilename = contextFileNames[0] ?? 'QWEN.md'; | ||||||
| const userMemoryFile = await resolvePreferredMemoryFile( | ||||||
| Storage.getGlobalQwenDir(), | ||||||
| fallbackFilename, | ||||||
| contextFileNames, | ||||||
| ); | ||||||
| const projectMemoryFile = await resolvePreferredMemoryFile( | ||||||
| params.cwd, | ||||||
| fallbackFilename, | ||||||
| contextFileNames, | ||||||
| ); | ||||||
| const autoMemoryDir = getAutoMemoryRoot(params.projectRoot); | ||||||
|
|
||||||
|
|
@@ -8148,7 +8160,11 @@ class QwenAgent implements Agent { | |||||
| ? params['projectRoot'] | ||||||
| : cwd; | ||||||
| return { | ||||||
| paths: await resolveQwenMemoryPaths({ cwd, projectRoot }), | ||||||
| paths: await resolveQwenMemoryPaths({ | ||||||
| cwd, | ||||||
| projectRoot, | ||||||
| contextFileNames: this.contextFileNamesForCwd(cwd), | ||||||
| }), | ||||||
| }; | ||||||
| } | ||||||
| case SERVE_STATUS_EXT_METHODS.workspaceMcp: | ||||||
|
|
@@ -10194,7 +10210,11 @@ class QwenAgent implements Agent { | |||||
| const relocation = await config.relocateWorkingDirectory( | ||||||
| canonicalPath, | ||||||
| canonicalPath, | ||||||
| { skipProcessChdir: true, skipArtifactMigration: true }, | ||||||
| { | ||||||
| skipProcessChdir: true, | ||||||
| skipArtifactMigration: true, | ||||||
| trustedFolder: true, | ||||||
| }, | ||||||
| ); | ||||||
| if (conversationDirectoryExpectation !== undefined) { | ||||||
| await assertManagedConversationDirectoryIdentity( | ||||||
|
|
@@ -10226,6 +10246,32 @@ class QwenAgent implements Agent { | |||||
| }`, | ||||||
| ); | ||||||
| } | ||||||
| for (const error of relocation.projectRuntimeRefreshErrors ?? []) { | ||||||
| warnings.push( | ||||||
| `Project runtime refresh failed: ${ | ||||||
| error instanceof Error ? error.message : String(error) | ||||||
| }`, | ||||||
| ); | ||||||
| } | ||||||
| if (relocation.cronExitSummary) { | ||||||
| warnings.push( | ||||||
| formatCronRelocationNotice(relocation.cronExitSummary), | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| await session.refreshSkillsFromSettings({ | ||||||
| reloadSettings: false, | ||||||
| notifyConfigChanged: false, | ||||||
| }); | ||||||
| } catch (error) { | ||||||
| warnings.push( | ||||||
| `Available commands refresh failed: ${ | ||||||
| error instanceof Error ? error.message : String(error) | ||||||
| }`, | ||||||
| ); | ||||||
| } | ||||||
| session.startCronScheduler(); | ||||||
|
|
||||||
| try { | ||||||
| await config | ||||||
|
|
@@ -12494,14 +12540,20 @@ class QwenAgent implements Agent { | |||||
| // not process.exit(1) the shared ACP child and every session on its | ||||||
| // channel. newSessionConfig maps the throw to a RequestError. | ||||||
| true, | ||||||
| this.managedToolInvocationGuard || restoreOptions || provisionalWorkspace | ||||||
| this.managedToolInvocationGuard || | ||||||
| restoreOptions || | ||||||
| provisionalWorkspace || | ||||||
| sessionSource?.sourceType === 'channel' | ||||||
| ? { | ||||||
| ...(provisionalWorkspace | ||||||
| ? { provisionalWorkspace: true as const } | ||||||
| : {}), | ||||||
| ...(this.managedToolInvocationGuard | ||||||
| ? { toolInvocationGuard: this.managedToolInvocationGuard } | ||||||
| : {}), | ||||||
| ...(sessionSource?.sourceType === 'channel' | ||||||
| ? { projectRuntimeCronEnabled: false } | ||||||
| : {}), | ||||||
| ...(restoreOptions && sessionId | ||||||
| ? { | ||||||
| sessionRestore: { | ||||||
|
|
@@ -12517,6 +12569,7 @@ class QwenAgent implements Agent { | |||||
| : {}), | ||||||
| } | ||||||
| : undefined, | ||||||
| settings, | ||||||
| ); | ||||||
| if (sessionSource) { | ||||||
| config.setSessionSource(sessionSource.sourceType, sessionSource.sourceId); | ||||||
|
|
@@ -12697,6 +12750,22 @@ class QwenAgent implements Agent { | |||||
| config.setFileSystemService(acpFileSystemService); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Context-file names for a host request about `cwd`. `/cd` scopes the | ||||||
| * names to the session and leaves the process-global list untouched, so | ||||||
| * an agent-level request has to be answered from the session that owns | ||||||
| * the directory; the global list is only right when no session does. | ||||||
| */ | ||||||
| private contextFileNamesForCwd(cwd: string): readonly string[] { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R2-3: still stands — the session-matching branch of 中文说明仍然成立—— — qwen3.8-max via Qwen Code /review (v0.22.2) |
||||||
| for (const session of this.sessions.values()) { | ||||||
| const config = session.getConfig(); | ||||||
| if (config.getWorkingDir() === cwd) { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R2-2: this lookup compares the raw host-supplied
Suggested change
(or Fix witness: an acpAgent test creating a session whose 中文说明该查找以严格字符串相等比较宿主提供的原始 修复见证:新增 acpAgent 测试:会话 — qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R2-2: still stands — 中文说明仍然成立—— — qwen3.8-max via Qwen Code /review (v0.22.2) |
||||||
| return config.getContextFileNames(); | ||||||
| } | ||||||
| } | ||||||
| return getAllMemoryFilenames(); | ||||||
| } | ||||||
|
|
||||||
| private async createAndStoreSession( | ||||||
| config: Config, | ||||||
| settings: LoadedSettings, | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1691,14 +1691,14 @@ export async function registerCreateSubSessionTool( | |
| } | ||
| const toolRegistry = config.getToolRegistry(); | ||
| if (registrationStatus === 'deferred') { | ||
| toolRegistry.registerPermissionDeferredFactory( | ||
| toolRegistry.registerSessionPermissionDeferredFactory( | ||
| ToolNames.CREATE_SUB_SESSION, | ||
|
Comment on lines
+1694
to
1695
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-12: round 1 flagged that Session's switch from Fix witness: assert 中文说明第 1 轮曾指出 Session 从 修复见证:在这些注册测试中直接断言 — qwen3.8-max via Qwen Code /review (v0.22.2) |
||
| async () => new CreateSubSessionTool(config), | ||
| ); | ||
| await config.getGeminiClient().setTools(); | ||
| return; | ||
| } | ||
| toolRegistry.registerTool(new CreateSubSessionTool(config)); | ||
| toolRegistry.registerSessionTool(new CreateSubSessionTool(config)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R1-12: still stands — Session's switch from 中文说明仍然成立——Session 从 — qwen3.8-max via Qwen Code /review (v0.22.2) |
||
| // The registration lands after `config.initialize()` → `startChat()` already | ||
| // snapshotted the chat's tool declarations, and the tool is deferred — so it | ||
| // stays filtered out of the declarations until revealed. Reveal it and | ||
|
|
@@ -3207,7 +3207,7 @@ export class Session implements SessionContext { | |
| screenshotPath, | ||
| }; | ||
| }); | ||
| registry.registerTool(tool); | ||
| registry.registerSessionTool(tool); | ||
| if (registry.getTool(CAPTURE_SCREEN_CONTEXT_TOOL_NAME) !== tool) { | ||
|
qqqys marked this conversation as resolved.
|
||
| throw new Error( | ||
| 'capture_screen_context is required for Live Voice but is disabled.', | ||
|
|
@@ -3231,7 +3231,7 @@ export class Session implements SessionContext { | |
| ); | ||
| } | ||
| } | ||
| for (const tool of tools) registry.registerTool(tool); | ||
| for (const tool of tools) registry.registerSessionTool(tool); | ||
| for (const tool of tools) { | ||
| if (registry.getTool(tool.name) !== tool) { | ||
| throw new Error( | ||
|
|
@@ -3258,7 +3258,7 @@ export class Session implements SessionContext { | |
| message, | ||
| }); | ||
| }); | ||
| registry.registerTool(tool); | ||
| registry.registerSessionTool(tool); | ||
| if (registry.getTool(SPEAK_TO_USER_TOOL_NAME) !== tool) { | ||
| throw new Error( | ||
| 'speak_to_user is required for Live Voice but is disabled.', | ||
|
|
@@ -10097,6 +10097,7 @@ export class Session implements SessionContext { | |
| const matchedContextFileWrite = didWriteProjectContextFile( | ||
| memoryWriteCandidates, | ||
| this.config.getProjectRoot(), | ||
| this.config.getContextFileNames(), | ||
| ); | ||
| debugLogger.debug( | ||
| `ACP session ${this.sessionId} checked marked context-file memory tool batch; matched=${matchedContextFileWrite}`, | ||
|
|
@@ -10969,7 +10970,11 @@ export class Session implements SessionContext { | |
| // prompt right after an allow-rule call just worked. | ||
| const forceAutoReviewForAllow = | ||
| approvalMode === ApprovalMode.AUTO && | ||
| (shouldForceAutoModeReviewForAllow(pmCtx, this.config.getCwd()) || | ||
| (shouldForceAutoModeReviewForAllow( | ||
| pmCtx, | ||
| this.config.getCwd(), | ||
| this.config.getContextFileNames(), | ||
| ) || | ||
| shouldClassifyAllShellForAutoMode(policyToolName, this.config)); | ||
| const confirmationPermission = getEffectivePermissionForConfirmation( | ||
| finalPermission, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R2-3: the session-matching branch of
contextFileNamesForCwd— the point of the change — is untested: the onlyqwen/settings/getMemoryPathstest in the tree (acpAgent.test.ts ~12029) queries a cwd that matches no live session, so only thereturn getAllMemoryFilenames()fallback executes. Deleting the session loop and always returning the global list keeps every existing test green while reintroducing the exact bug the doc comment names (a host asking for memory paths about a moved session's cwd is handedQWEN.mdfor a project whose context file isCONTEXT.md).Fix witness: add a test where a session owns the queried cwd with
getContextFileNames()['CONTEXT.md']and assert the resolved paths useCONTEXT.md— it must go red if the session loop is removed.中文说明
contextFileNamesForCwd的会话匹配分支——本次改动的核心——没有测试:树中唯一的qwen/settings/getMemoryPaths测试(acpAgent.test.ts ~12029)查询的 cwd 不属于任何活跃会话,因此只有return getAllMemoryFilenames()回退分支被执行。删除会话循环、总是返回全局列表,所有现有测试仍为绿,却重新引入了文档注释所指明的 bug(宿主查询已迁移会话的 cwd 时,项目上下文文件明明是CONTEXT.md却拿到QWEN.md)。修复见证:新增测试,让会话持有被查询的 cwd 且
getContextFileNames()为['CONTEXT.md'],断言解析路径使用CONTEXT.md;删除会话循环后必须变红。— qwen3.8-max via Qwen Code /review (v0.22.2)