-
Notifications
You must be signed in to change notification settings - Fork 124
BOT-1571: Clarify stable project IDs after project rename #341
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 1 commit
885e64f
c75a215
47e591f
2029814
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 |
|---|---|---|
|
|
@@ -8,6 +8,8 @@ interface GetContextResult { | |
| view: string; | ||
| active_session_id: string | null; | ||
| active_project_id: string | null; | ||
| /** Current display name of the active project, or null. */ | ||
| active_project_name: string | null; | ||
| voice_session_active: boolean; | ||
| app_version: string; | ||
| } | ||
|
|
@@ -41,29 +43,42 @@ export const getContextCommand = defineCommand({ | |
|
|
||
| Result: | ||
| {"view": "...", "active_session_id": "..."|null, | ||
| "active_project_id": "..."|null, "voice_session_active": true|false, | ||
| "app_version": "..."}`, | ||
| "active_project_id": "..."|null, "active_project_name": "..."|null, | ||
| "voice_session_active": true|false, "app_version": "..."} | ||
|
|
||
| "active_project_id" is a stable identifier that does not change when the | ||
| project is renamed; "active_project_name" is the project's current display | ||
| name and may not match the id (e.g. "goose-internal" / "Berd").`, | ||
| schema: getContextSchema, | ||
| execute: async (): Promise<GetContextResult> => { | ||
| const [ | ||
| { default: packageJson }, | ||
| { getAppNavigationController }, | ||
| { getVoiceConversationStatus }, | ||
| { useVoiceConversationStore }, | ||
| { useProjectStore }, | ||
| ] = await Promise.all([ | ||
| import("../../../../../package.json"), | ||
| import("../../navigation"), | ||
| import("@/features/voice-conversation/api/voiceConversation"), | ||
| import("@/features/voice-conversation/stores/voiceConversationStore"), | ||
| import("@/features/projects/stores/projectStore"), | ||
| ]); | ||
| const context = getAppNavigationController().getAppContext(); | ||
| const voiceBeforeRefresh = useVoiceConversationStore.getState(); | ||
| const nativeVoiceStatus = await getVoiceConversationStatus(); | ||
| const voiceAfterRefresh = useVoiceConversationStore.getState(); | ||
| const activeProjectName = context.activeProjectId | ||
|
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. 🤖 P1 · Resolve the current project name (blocking) The new field is derived only from useProjectStore.getState().projects. That store starts from an optional localStorage seed and may be empty before projects load or stale after another writer renames the project, so a valid active project ID can be paired with null or an old display name. This defeats the field's stated purpose of disambiguating stable IDs after rename. User effect: Agents can still mistake the active project for a different project, or use an outdated project name, even though the command now promises the current display name. Recommended fix: Resolve the active project through the existing current-data project loader before reading its name, while preserving the captured active project ID for a consistent response; return null only when there is no active project or the current project genuinely cannot be found. Test: Add discriminating command tests where the store is initially empty and where it contains a stale name while the backend returns the renamed project; assert that the result uses the backend's current name and keeps the same stable ID.
Contributor
Author
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. 🤖 Addressed in c75a215. Added command-level regressions for stale cached names, an empty initial store, archived active projects, and no active project. These assert the stable ID is preserved and the returned name is current. |
||
| ? (useProjectStore | ||
| .getState() | ||
| .projects.find((project) => project.id === context.activeProjectId) | ||
| ?.name ?? null) | ||
| : null; | ||
| return { | ||
| view: context.view, | ||
| active_session_id: context.activeSessionId, | ||
| active_project_id: context.activeProjectId, | ||
| active_project_name: activeProjectName, | ||
| voice_session_active: | ||
| nativeVoiceStatus.sessionId !== null || | ||
| rendererVoiceSessionActive( | ||
|
|
||
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.
🤖 P2 · Report every duplicate directory (non-blocking)
The command accepts multiple working_dir values but stops scanning after the first matching project. The returned warning therefore omits any additional directories already attached to other active projects, even though each omitted conflict carries the same risk the warning is intended to expose.
User effect: An agent can believe it has seen the full conflict and continue with a project that still splits related chats and history across other existing projects.
Recommended fix: Collect every conflicting directory and project before creation and return all conflicts, preferably in a structured result that agents can inspect reliably.
Test: Add a command test with multiple requested directories that conflict with different active projects and assert that every conflict is returned.