diff --git a/cmd/sgai/mcp_external.go b/cmd/sgai/mcp_external.go index d355786..21b23cc 100644 --- a/cmd/sgai/mcp_external.go +++ b/cmd/sgai/mcp_external.go @@ -5,14 +5,17 @@ import ( "errors" "fmt" "log" + "maps" "net" "net/http" "net/url" "os" "path/filepath" + "slices" "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/ucirello/sgai/pkg/state" ) type externalMCPContext struct { @@ -21,28 +24,46 @@ type externalMCPContext struct { } type externalTargetArgs struct { - Target string `json:"target" jsonschema:"Repository or workspace handle, basename, or absolute path returned by factory_info."` + Target string `json:"target" jsonschema:"Repository or workspace label, basename, or absolute path returned by factory_info."` } type externalForkArgs struct { - Target string `json:"target" jsonschema:"Repository handle, basename, or absolute path returned by factory_info."` - GoalContent string `json:"goalContent" jsonschema:"GOAL.md content for the new fork. Provide the full document, including frontmatter when needed."` + Target string `json:"target" jsonschema:"Repository label, basename, or absolute path returned by factory_info."` + GoalContent string `json:"goalContent" jsonschema:"Body-only GOAL.md content for the new fork. The backend copies the parent frontmatter and appends this body text after it."` + Title string `json:"title" jsonschema:"Required title for external fork creation. This overrides only the copied title field in the new fork GOAL.md frontmatter."` } type externalAttachArgs struct { Path string `json:"path" jsonschema:"Absolute on-disk path to an existing repository or workspace directory."` } +type externalWorkspaceArgs struct { + Workspace string `json:"workspace" jsonschema:"Workspace label, basename, or absolute path returned by factory_info."` +} + +type externalAnswerPendingQuestionArgs struct { + Workspace string `json:"workspace" jsonschema:"Workspace label, basename, or absolute path returned by factory_info."` + PromptToken string `json:"promptToken" jsonschema:"Current prompt token returned by get_pending_question."` + Answer string `json:"answer" jsonschema:"Optional free-text answer for the current pending question. Provide answer text, selected choices, or both."` + SelectedChoices []string `json:"selectedChoices" jsonschema:"Optional selected choices for the current pending question. Provide selected choices, answer text, or both."` +} + +type externalSteerNextTurnArgs struct { + Workspace string `json:"workspace" jsonschema:"Workspace label, basename, or absolute path returned by factory_info."` + Message string `json:"message" jsonschema:"Free-text re-steering instruction for the next turn."` +} + type externalRepositoryInfo struct { - Handle string `json:"handle"` - DirectoryName string `json:"directoryName"` - Label string `json:"label"` - Title string `json:"title,omitempty"` - Path string `json:"path"` - Mode string `json:"mode"` - RootHandle string `json:"rootHandle,omitempty"` - RootPath string `json:"rootPath,omitempty"` - ForkCount int `json:"forkCount,omitempty"` + Handle string `json:"handle"` + DirectoryName string `json:"directoryName"` + Label string `json:"label"` + Title string `json:"title,omitempty"` + Path string `json:"path"` + Mode string `json:"mode"` + RootHandle string `json:"rootHandle,omitempty"` + RootPath string `json:"rootPath,omitempty"` + ForkCount int `json:"forkCount,omitempty"` + ActiveAgents []string `json:"activeAgents"` } type externalFactoryInfoResult struct { @@ -57,6 +78,7 @@ type externalSessionActionResult struct { Running bool `json:"running"` Message string `json:"message"` AlreadyRunning bool `json:"alreadyRunning,omitempty"` + RunningMode string `json:"runningMode,omitempty"` } type externalGoalEditLinkResult struct { @@ -64,6 +86,17 @@ type externalGoalEditLinkResult struct { URL string `json:"url"` } +type externalWorkflowLinks struct { + Progress string `json:"progress"` + GoalEdit string `json:"goalEdit"` + Respond string `json:"respond,omitempty"` +} + +type externalWorkflowLinksResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + Links externalWorkflowLinks `json:"links"` +} + type externalAttachResult struct { Workspace externalRepositoryInfo `json:"workspace"` HasGoal bool `json:"hasGoal"` @@ -77,6 +110,44 @@ type externalForkResult struct { Message string `json:"message"` } +type externalPendingQuestionListResult struct { + Workspaces []externalRepositoryInfo `json:"workspaces"` +} + +type externalPendingQuestionResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + PendingQuestion apiPendingQuestionResponse `json:"pendingQuestion"` +} + +type externalAnswerPendingQuestionResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + Success bool `json:"success"` + Message string `json:"message"` +} + +type externalSteerNextTurnResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + Success bool `json:"success"` + Message string `json:"message"` +} + +type externalWorkspaceLogResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + Log []apiLogEntry `json:"log"` +} + +type externalAgentTodoSection struct { + Agent string `json:"agent"` + Todos []apiTodoEntry `json:"todos"` +} + +type externalWorkspaceMessagesAndTodosResult struct { + Workspace externalRepositoryInfo `json:"workspace"` + Messages []state.Message `json:"messages"` + ProjectTodos []apiTodoEntry `json:"projectTodos"` + ActiveAgentTodoSections []externalAgentTodoSection `json:"activeAgentTodoSections"` +} + func buildExternalMCPHandler(server *Server) (http.Handler, error) { if _, errBuild := buildExternalMCPServer(server, nil); errBuild != nil { return nil, errBuild @@ -109,6 +180,14 @@ func registerExternalTools(server *mcp.Server, mcpCtx *externalMCPContext) error if errSchema != nil { return errSchema } + startInteractiveSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("start_interactive") + if errSchema != nil { + return errSchema + } + startContinuousSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("start_continuous") + if errSchema != nil { + return errSchema + } stopWorkspaceSchema, errSchema := buildToolSchema[externalTargetArgs]("stop_workspace") if errSchema != nil { return errSchema @@ -129,14 +208,51 @@ func registerExternalTools(server *mcp.Server, mcpCtx *externalMCPContext) error if errSchema != nil { return errSchema } + listPendingQuestionsSchema, errSchema := buildToolSchema[struct{}]("list_pending_questions") + if errSchema != nil { + return errSchema + } + getPendingQuestionSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("get_pending_question") + if errSchema != nil { + return errSchema + } + answerPendingQuestionSchema, errSchema := buildToolSchema[externalAnswerPendingQuestionArgs]("answer_pending_question") + if errSchema != nil { + return errSchema + } + steerNextTurnSchema, errSchema := buildToolSchema[externalSteerNextTurnArgs]("steer_next_turn") + if errSchema != nil { + return errSchema + } + peekWorkspaceLogSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("peek_workspace_log") + if errSchema != nil { + return errSchema + } + workflowLinksSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("workflow_links") + if errSchema != nil { + return errSchema + } + workspaceMessagesAndTodosSchema, errSchema := buildToolSchema[externalWorkspaceArgs]("workspace_messages_and_todos") + if errSchema != nil { + return errSchema + } - mcp.AddTool(server, newMCPTool("factory_info", "Describe this factory, including hostname, start directory, and attached repositories with hybrid handles and modes.", factoryInfoSchema), mcpCtx.factoryInfoHandler) + mcp.AddTool(server, newMCPTool("factory_info", "Describe this factory, including hostname, start directory, and attached repositories with authoritative workspace labels and modes.", factoryInfoSchema), mcpCtx.factoryInfoHandler) mcp.AddTool(server, newMCPTool("start_self_drive", "Start an attached repository or workspace in Self-Drive Mode.", startSelfDriveSchema), mcpCtx.startSelfDriveHandler) + mcp.AddTool(server, newMCPTool("start_interactive", "Start an attached workspace in interactive mode. Continuous-configured workspaces are rejected and re-steered to start_continuous.", startInteractiveSchema), mcpCtx.startInteractiveHandler) + mcp.AddTool(server, newMCPTool("start_continuous", "Start an attached workspace in continuous mode. Workspaces without continuous configuration are rejected and re-steered.", startContinuousSchema), mcpCtx.startContinuousHandler) mcp.AddTool(server, newMCPTool("stop_workspace", "Stop an attached workspace. This is idempotent for already stopped workspaces.", stopWorkspaceSchema), mcpCtx.stopWorkspaceHandler) mcp.AddTool(server, newMCPTool("reset_workspace", "Reset a stopped workspace state. Running workspaces must be stopped before reset.", resetWorkspaceSchema), mcpCtx.resetWorkspaceHandler) mcp.AddTool(server, newMCPTool("goal_edit_link", "Return a browser URL for editing GOAL.md for the selected workspace, based on the host used for this MCP connection.", goalEditLinkSchema), mcpCtx.goalEditLinkHandler) - mcp.AddTool(server, newMCPTool("fork_repository", "Fork a standalone repository or root repository and create the new fork with the supplied GOAL.md content.", forkRepositorySchema), mcpCtx.forkRepositoryHandler) + mcp.AddTool(server, newMCPTool("fork_repository", "Fork a standalone repository or root repository. The new fork copies the parent GOAL frontmatter, appends the supplied body-only goal content, and requires an explicit title override.", forkRepositorySchema), mcpCtx.forkRepositoryHandler) mcp.AddTool(server, newMCPTool("attach_repository", "Attach an existing on-disk repository or workspace by absolute path.", attachRepositorySchema), mcpCtx.attachRepositoryHandler) + mcp.AddTool(server, newMCPTool("list_pending_questions", "List attached workspaces that currently have live pending questions.", listPendingQuestionsSchema), mcpCtx.listPendingQuestionsHandler) + mcp.AddTool(server, newMCPTool("get_pending_question", "Return the full pending-question payload for the selected workspace.", getPendingQuestionSchema), mcpCtx.getPendingQuestionHandler) + mcp.AddTool(server, newMCPTool("answer_pending_question", "Answer the current pending question for a workspace using the current prompt token, free-text answer, selected choices, or both.", answerPendingQuestionSchema), mcpCtx.answerPendingQuestionHandler) + mcp.AddTool(server, newMCPTool("steer_next_turn", "Add a Session-tab-style re-steering instruction for the selected workspace.", steerNextTurnSchema), mcpCtx.steerNextTurnHandler) + mcp.AddTool(server, newMCPTool("peek_workspace_log", "Peek the live Log-tab-equivalent buffer for the selected workspace.", peekWorkspaceLogSchema), mcpCtx.peekWorkspaceLogHandler) + mcp.AddTool(server, newMCPTool("workflow_links", "Return the minimal browser workflow links for the selected workspace: progress, goal-edit, and respond when the workspace currently needs input.", workflowLinksSchema), mcpCtx.workflowLinksHandler) + mcp.AddTool(server, newMCPTool("workspace_messages_and_todos", "Export workspace messages in internal MCP order plus project TODOs and the currently active agents' TODOs.", workspaceMessagesAndTodosSchema), mcpCtx.workspaceMessagesAndTodosHandler) return nil } @@ -165,18 +281,40 @@ func (c *externalMCPContext) startSelfDriveHandler(_ context.Context, _ *mcp.Cal return nil, externalSessionActionResult{}, errResolve } - result, errStart := c.server.startSessionService(workspacePath, true) + result, errStart := c.server.startSessionInModeService(workspacePath, sessionStartModeSelfDrive) if errStart != nil { return nil, externalSessionActionResult{}, humanizeStartSelfDriveError(&workspace, errStart) } - return nil, externalSessionActionResult{ - Workspace: workspace, - Status: result.Status, - Running: result.Running, - Message: result.Message, - AlreadyRunning: result.AlreadyRunning, - }, nil + return nil, externalStartSessionActionResult(&workspace, &result), nil +} + +func (c *externalMCPContext) startInteractiveHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalSessionActionResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalSessionActionResult{}, errResolve + } + + result, errStart := c.server.startSessionInModeService(workspacePath, sessionStartModeInteractive) + if errStart != nil { + return nil, externalSessionActionResult{}, humanizeStartInteractiveError(&workspace, errStart) + } + + return nil, externalStartSessionActionResult(&workspace, &result), nil +} + +func (c *externalMCPContext) startContinuousHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalSessionActionResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalSessionActionResult{}, errResolve + } + + result, errStart := c.server.startSessionInModeService(workspacePath, sessionStartModeContinuous) + if errStart != nil { + return nil, externalSessionActionResult{}, humanizeStartContinuousError(&workspace, errStart) + } + + return nil, externalStartSessionActionResult(&workspace, &result), nil } func (c *externalMCPContext) stopWorkspaceHandler(_ context.Context, _ *mcp.CallToolRequest, args externalTargetArgs) (*mcp.CallToolResult, externalSessionActionResult, error) { @@ -193,6 +331,7 @@ func (c *externalMCPContext) stopWorkspaceHandler(_ context.Context, _ *mcp.Call Running: result.Running, Message: result.Message, AlreadyRunning: false, + RunningMode: "", }, nil } @@ -213,6 +352,7 @@ func (c *externalMCPContext) resetWorkspaceHandler(_ context.Context, _ *mcp.Cal Running: result.Running, Message: result.Message, AlreadyRunning: false, + RunningMode: "", }, nil } @@ -222,25 +362,24 @@ func (c *externalMCPContext) goalEditLinkHandler(_ context.Context, _ *mcp.CallT return nil, externalGoalEditLinkResult{}, errResolve } - baseURL := externalBaseURL(c.request) - workspaceName, errWorkspaceName := c.goalEditWorkspaceName(&workspace) + workspaceName, errWorkspaceName := c.workflowLinkWorkspaceName(&workspace, "goal edit link") if errWorkspaceName != nil { return nil, externalGoalEditLinkResult{}, errWorkspaceName } - goalURL, errGoalURL := externalGoalEditURL(baseURL, workspaceName) - if errGoalURL != nil { - return nil, externalGoalEditLinkResult{}, errGoalURL + goalEditURL, errGoalEditURL := externalGoalEditURL(externalBaseURL(c.request), workspaceName) + if errGoalEditURL != nil { + return nil, externalGoalEditLinkResult{}, errGoalEditURL } - return nil, externalGoalEditLinkResult{Workspace: workspace, URL: goalURL}, nil + return nil, externalGoalEditLinkResult{Workspace: workspace, URL: goalEditURL}, nil } -func (c *externalMCPContext) goalEditWorkspaceName(workspace *externalRepositoryInfo) (string, error) { +func (c *externalMCPContext) workflowLinkWorkspaceName(workspace *externalRepositoryInfo, linkSet string) (string, error) { matches := c.server.resolveWorkspaceNameToPaths(workspace.DirectoryName) if len(matches) == 1 && sameWorkspacePath(matches[0], workspace.Path) { return workspace.DirectoryName, nil } - return "", fmt.Errorf("cannot build goal edit link for %q because workspace basename %q is ambiguous across attached repositories", workspace.Handle, workspace.DirectoryName) + return "", fmt.Errorf("cannot build %s for %q because workspace basename %q is ambiguous across attached repositories", linkSet, workspace.Handle, workspace.DirectoryName) } func (c *externalMCPContext) forkRepositoryHandler(_ context.Context, _ *mcp.CallToolRequest, args externalForkArgs) (*mcp.CallToolResult, externalForkResult, error) { @@ -249,7 +388,7 @@ func (c *externalMCPContext) forkRepositoryHandler(_ context.Context, _ *mcp.Cal return nil, externalForkResult{}, errResolve } - result, errFork := c.server.forkWorkspaceService(workspacePath, args.GoalContent) + result, errFork := c.server.forkWorkspaceServiceWithOptions(workspacePath, args.GoalContent, forkWorkspaceOptions{title: args.Title, requireTitle: true}) if errFork != nil { return nil, externalForkResult{}, humanizeForkRepositoryError(&workspace, errFork) } @@ -285,17 +424,187 @@ func (c *externalMCPContext) attachRepositoryHandler(_ context.Context, _ *mcp.C }, nil } +func (c *externalMCPContext) listPendingQuestionsHandler(_ context.Context, _ *mcp.CallToolRequest, _ struct{}) (*mcp.CallToolResult, externalPendingQuestionListResult, error) { + repositories, errRepositories := c.repositories() + if errRepositories != nil { + return nil, externalPendingQuestionListResult{}, errRepositories + } + + workspaces := make([]externalRepositoryInfo, 0) + for i := range repositories { + repository := repositories[i] + if _, errPendingQuestion := c.livePendingQuestion(repository.Path); errPendingQuestion == nil { + workspaces = append(workspaces, repository) + } + } + + return nil, externalPendingQuestionListResult{Workspaces: workspaces}, nil +} + +func (c *externalMCPContext) getPendingQuestionHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalPendingQuestionResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalPendingQuestionResult{}, errResolve + } + + pendingQuestion, errPendingQuestion := c.livePendingQuestion(workspacePath) + if errPendingQuestion != nil { + return nil, externalPendingQuestionResult{}, humanizeGetPendingQuestionError(&workspace, errPendingQuestion) + } + + return nil, externalPendingQuestionResult{Workspace: workspace, PendingQuestion: *pendingQuestion}, nil +} + +func (c *externalMCPContext) answerPendingQuestionHandler(_ context.Context, _ *mcp.CallToolRequest, args externalAnswerPendingQuestionArgs) (*mcp.CallToolResult, externalAnswerPendingQuestionResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalAnswerPendingQuestionResult{}, errResolve + } + + result, errRespond := c.server.respondService(workspacePath, args.PromptToken, args.Answer, args.SelectedChoices) + if errRespond != nil { + return nil, externalAnswerPendingQuestionResult{}, humanizeAnswerPendingQuestionError(&workspace, errRespond) + } + + return nil, externalAnswerPendingQuestionResult{Workspace: workspace, Success: result.Success, Message: result.Message}, nil +} + +func (c *externalMCPContext) steerNextTurnHandler(_ context.Context, _ *mcp.CallToolRequest, args externalSteerNextTurnArgs) (*mcp.CallToolResult, externalSteerNextTurnResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalSteerNextTurnResult{}, errResolve + } + + result, errSteer := c.server.steerService(workspacePath, args.Message) + if errSteer != nil { + return nil, externalSteerNextTurnResult{}, humanizeSteerNextTurnError(&workspace, errSteer) + } + + return nil, externalSteerNextTurnResult{Workspace: workspace, Success: result.Success, Message: result.Message}, nil +} + +func (c *externalMCPContext) workflowLinksHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalWorkflowLinksResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalWorkflowLinksResult{}, errResolve + } + + links, errWorkflowLinks := c.workflowLinks(&workspace, workspacePath) + if errWorkflowLinks != nil { + return nil, externalWorkflowLinksResult{}, errWorkflowLinks + } + + return nil, externalWorkflowLinksResult{Workspace: workspace, Links: links}, nil +} + +func (c *externalMCPContext) peekWorkspaceLogHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalWorkspaceLogResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalWorkspaceLogResult{}, errResolve + } + + return nil, externalWorkspaceLogResult{Workspace: workspace, Log: c.workspaceLogEntries(workspacePath)}, nil +} + +func (c *externalMCPContext) workspaceMessagesAndTodosHandler(_ context.Context, _ *mcp.CallToolRequest, args externalWorkspaceArgs) (*mcp.CallToolResult, externalWorkspaceMessagesAndTodosResult, error) { + workspace, workspacePath, errResolve := c.resolveTarget(args.Workspace) + if errResolve != nil { + return nil, externalWorkspaceMessagesAndTodosResult{}, errResolve + } + + wfState := c.server.workflowStateSnapshot(workspacePath) + activeAgents := c.server.activeAgentsForWorkspace(workspacePath) + sections := make([]externalAgentTodoSection, 0, len(activeAgents)) + for _, agent := range activeAgents { + sections = append(sections, externalAgentTodoSection{ + Agent: agent, + Todos: convertTodosForAPI(wfState.TodosByAgent[agent]), + }) + } + + return nil, externalWorkspaceMessagesAndTodosResult{ + Workspace: workspace, + Messages: slices.Clone(wfState.Messages), + ProjectTodos: convertTodosForAPI(wfState.ProjectTodos), + ActiveAgentTodoSections: sections, + }, nil +} + +func externalStartSessionActionResult(workspace *externalRepositoryInfo, result *sessionStartResult) externalSessionActionResult { + message := result.Message + if result.RunningMode != "" { + if result.AlreadyRunning { + message = "workspace already running in " + result.RunningMode + " mode" + } else { + message = "workspace started in " + result.RunningMode + " mode" + } + } + return externalSessionActionResult{ + Workspace: *workspace, + Status: result.Status, + Running: result.Running, + Message: message, + AlreadyRunning: result.AlreadyRunning, + RunningMode: result.RunningMode, + } +} + +func (c *externalMCPContext) livePendingQuestion(workspacePath string) (*apiPendingQuestionResponse, error) { + coord := c.server.sessionCoordinator(workspacePath) + if coord == nil { + return nil, errNoPendingQuestion + } + humanInput := currentHumanInputSnapshot(coord) + if !humanInput.needsInput() { + return nil, errNoPendingQuestion + } + return humanInput.pendingQuestion(coord.State().CurrentAgent), nil +} + +func (c *externalMCPContext) workspaceLogEntries(workspacePath string) []apiLogEntry { + c.server.mu.Lock() + sess := c.server.sessions[workspacePath] + c.server.mu.Unlock() + if sess == nil || sess.outputLog == nil { + return nil + } + entries := make([]apiLogEntry, 0) + for _, line := range sess.outputLog.lines() { + entries = append(entries, apiLogEntry{Prefix: line.prefix, Text: line.text}) + } + return entries +} + +func (s *Server) workflowStateSnapshot(workspacePath string) state.Workflow { + if coord := s.sessionCoordinator(workspacePath); coord != nil { + return coord.State() + } + return s.loadWorkspaceState(workspacePath) +} + +func (s *Server) activeAgentsForWorkspace(workspacePath string) []string { + if !s.sessionRunning(workspacePath) { + return []string{} + } + activeAgents := splitCurrentAgents(s.workflowStateSnapshot(workspacePath).CurrentAgent) + if activeAgents == nil { + return []string{} + } + return activeAgents +} + func (c *externalMCPContext) repositories() ([]externalRepositoryInfo, error) { groups, errGroups := c.workspaceGroups() if errGroups != nil { return nil, errGroups } + displayLabels := c.displayLabelsForGroups(groups) var repositories []externalRepositoryInfo for _, group := range groups { - repositories = append(repositories, c.repositoryInfo(group.Root, groups)) + repositories = append(repositories, c.repositoryInfo(group.Root, groups, displayLabels)) for _, fork := range group.Forks { - repositories = append(repositories, c.repositoryInfo(fork, groups)) + repositories = append(repositories, c.repositoryInfo(fork, groups, displayLabels)) } } @@ -316,7 +625,7 @@ func (c *externalMCPContext) workspaceGroups() ([]workspaceGroup, error) { func (c *externalMCPContext) resolveTarget(target string) (externalRepositoryInfo, string, error) { target = strings.TrimSpace(target) if target == "" { - return externalRepositoryInfo{}, "", errors.New("target is required. Call factory_info and use a returned handle, basename, or absolute path") + return externalRepositoryInfo{}, "", errors.New("target is required. Call factory_info and use a returned label, basename, or absolute path") } repositories, errRepositories := c.repositories() @@ -331,24 +640,24 @@ func (c *externalMCPContext) resolveTarget(target string) (externalRepositoryInf return repository, repository.Path, nil } } - return externalRepositoryInfo{}, "", fmt.Errorf("repository %q is not attached. Call factory_info and use one of the returned handle, basename, or absolute path values", target) + return externalRepositoryInfo{}, "", fmt.Errorf("repository %q is not attached. Call factory_info and use one of the returned label, basename, or absolute path values", target) } var matches []externalRepositoryInfo for i := range repositories { repository := repositories[i] - if repository.Handle == target || repository.DirectoryName == target { + if repository.Handle == target || repository.Label == target || repository.DirectoryName == target { matches = append(matches, repository) } } switch len(matches) { case 0: - return externalRepositoryInfo{}, "", fmt.Errorf("repository %q not found. Call factory_info and use one of the returned handle, basename, or absolute path values", target) + return externalRepositoryInfo{}, "", fmt.Errorf("repository %q not found. Call factory_info and use one of the returned label, basename, or absolute path values", target) case 1: return matches[0], matches[0].Path, nil default: - return externalRepositoryInfo{}, "", fmt.Errorf("target %q matches multiple attached repositories. Use the exact handle or absolute path returned by factory_info", target) + return externalRepositoryInfo{}, "", fmt.Errorf("target %q matches multiple attached repositories. Use the exact label, basename, or absolute path returned by factory_info", target) } } @@ -368,11 +677,11 @@ func (c *externalMCPContext) repositoryByPath(workspacePath string) (externalRep return externalRepositoryInfo{}, fmt.Errorf("attached repository %q not found after update", workspacePath) } -func (c *externalMCPContext) repositoryInfo(workspace workspaceInfo, groups []workspaceGroup) externalRepositoryInfo { - label := repositoryLabel(workspace, groups) +func (c *externalMCPContext) repositoryInfo(workspace workspaceInfo, groups []workspaceGroup, displayLabels map[string]string) externalRepositoryInfo { + label := repositoryDisplayLabel(workspace, displayLabels) title := goalTitleStateFromPath(workspace.Directory, workspace.DirName).Title repository := externalRepositoryInfo{ - Handle: repositoryHandle(label, workspace.DirName), + Handle: label, DirectoryName: workspace.DirName, Label: label, Title: title, @@ -381,6 +690,7 @@ func (c *externalMCPContext) repositoryInfo(workspace workspaceInfo, groups []wo RootHandle: "", RootPath: "", ForkCount: 0, + ActiveAgents: c.server.activeAgentsForWorkspace(workspace.Directory), } if repository.Mode == string(workspaceRoot) { @@ -388,48 +698,166 @@ func (c *externalMCPContext) repositoryInfo(workspace workspaceInfo, groups []wo } if rootWorkspace, ok := rootWorkspaceForFork(workspace.Directory, groups); ok { - rootLabel := repositoryLabel(rootWorkspace, groups) - repository.RootHandle = repositoryHandle(rootLabel, rootWorkspace.DirName) + repository.RootHandle = repositoryDisplayLabel(rootWorkspace, displayLabels) repository.RootPath = rootWorkspace.Directory } return repository } -func repositoryMode(workspace workspaceInfo, groups []workspaceGroup) string { - if workspace.Kind != "" { - return string(workspace.Kind) +type externalWorkspaceDisplayIdentity struct { + Name string + Dir string + Title string + ComputedTitle string +} + +func (c *externalMCPContext) displayLabelsForGroups(groups []workspaceGroup) map[string]string { + identities := make([]externalWorkspaceDisplayIdentity, 0) + for _, group := range groups { + identities = append(identities, c.displayIdentity(group.Root, groups)) + for _, fork := range group.Forks { + identities = append(identities, c.displayIdentity(fork, groups)) + } } - if _, ok := rootWorkspaceForFork(workspace.Directory, groups); ok { - return string(workspaceFork) + disambiguators := externalWorkspaceNameDisambiguators(identities) + labels := make(map[string]string, len(identities)) + for _, identity := range identities { + labels[identity.Dir] = externalWorkspaceDisplayLabel(identity, disambiguators) } - if workspace.IsRoot { - return string(workspaceRoot) + return labels +} + +func (c *externalMCPContext) displayIdentity(workspace workspaceInfo, groups []workspaceGroup) externalWorkspaceDisplayIdentity { + entry := c.server.buildWorkspaceListEntry(workspace, groups) + return externalWorkspaceDisplayIdentity{ + Name: entry.Name, + Dir: entry.Dir, + Title: entry.Title, + ComputedTitle: entry.ComputedTitle, } - return string(workspaceStandalone) } -func repositoryLabel(workspace workspaceInfo, groups []workspaceGroup) string { - label := titleLabelForWorkspace(workspace) - if rootWorkspace, ok := rootWorkspaceForFork(workspace.Directory, groups); ok { - return titleLabelForWorkspace(rootWorkspace) + "/" + label +func externalWorkspaceDisplayLabel(identity externalWorkspaceDisplayIdentity, disambiguators map[string]string) string { + baseLabel := externalWorkspaceBaseLabel(identity) + disambiguator := disambiguators[identity.Dir] + if disambiguator == "" { + return baseLabel } - return label + return baseLabel + " · " + disambiguator } -func titleLabelForWorkspace(workspace workspaceInfo) string { - label := goalTitleStateFromPath(workspace.Directory, workspace.DirName).label() - if label == "" { - return workspace.DirName +func externalWorkspaceBaseLabel(identity externalWorkspaceDisplayIdentity) string { + computedTitle := strings.TrimSpace(identity.ComputedTitle) + if computedTitle != "" { + return computedTitle } - return label + title := strings.TrimSpace(identity.Title) + if title != "" { + return title + } + return identity.Name +} + +func repositoryDisplayLabel(workspace workspaceInfo, displayLabels map[string]string) string { + if label := displayLabels[workspace.Directory]; label != "" { + return label + } + return workspace.DirName } -func repositoryHandle(label, dirName string) string { - if label == "" || label == dirName { - return dirName +func externalWorkspaceNameDisambiguators(identities []externalWorkspaceDisplayIdentity) map[string]string { + grouped := make(map[string][]externalWorkspaceDisplayIdentity) + for _, identity := range identities { + grouped[identity.Name] = append(grouped[identity.Name], identity) } - return label + " [" + dirName + "]" + disambiguators := make(map[string]string) + for _, group := range grouped { + if len(group) < 2 { + continue + } + maps.Copy(disambiguators, externalGroupDisambiguators(group)) + } + return disambiguators +} + +func externalGroupDisambiguators(identities []externalWorkspaceDisplayIdentity) map[string]string { + type parentSegments struct { + dir string + segments []string + } + parents := make([]parentSegments, 0, len(identities)) + maxDepth := 0 + for _, identity := range identities { + segments := externalSplitPathSegments(identity.Dir) + if len(segments) > 0 { + segments = segments[:len(segments)-1] + } + parents = append(parents, parentSegments{dir: identity.Dir, segments: segments}) + if len(segments) > maxDepth { + maxDepth = len(segments) + } + } + if maxDepth == 0 { + maxDepth = 1 + } + disambiguators := make(map[string]string, len(parents)) + for _, current := range parents { + resolved := current.dir + for depth := 1; depth <= maxDepth; depth++ { + candidate := strings.Join(lastPathSegments(current.segments, depth), "/") + normalizedCandidate := candidate + if normalizedCandidate == "" { + normalizedCandidate = current.dir + } + unique := true + for _, other := range parents { + if other.dir == current.dir { + continue + } + otherCandidate := strings.Join(lastPathSegments(other.segments, depth), "/") + if otherCandidate == "" { + otherCandidate = other.dir + } + if otherCandidate == normalizedCandidate { + unique = false + break + } + } + if unique { + resolved = normalizedCandidate + break + } + } + disambiguators[current.dir] = resolved + } + return disambiguators +} + +func externalSplitPathSegments(path string) []string { + return strings.FieldsFunc(path, func(r rune) bool { + return r == '/' || r == '\\' + }) +} + +func lastPathSegments(segments []string, count int) []string { + if count >= len(segments) { + return segments + } + return segments[len(segments)-count:] +} + +func repositoryMode(workspace workspaceInfo, groups []workspaceGroup) string { + if workspace.Kind != "" { + return string(workspace.Kind) + } + if _, ok := rootWorkspaceForFork(workspace.Directory, groups); ok { + return string(workspaceFork) + } + if workspace.IsRoot { + return string(workspaceRoot) + } + return string(workspaceStandalone) } func forkCount(rootPath string, groups []workspaceGroup) int { @@ -460,6 +888,57 @@ func humanizeStartSelfDriveError(workspace *externalRepositoryInfo, err error) e return err } +func humanizeStartInteractiveError(workspace *externalRepositoryInfo, err error) error { + switch { + case errors.Is(err, errRootWorkspaceCannotStart): + return fmt.Errorf("cannot start %q in interactive mode because it is a root repository. Choose a standalone repository or a fork workspace instead", workspace.Handle) + case errors.Is(err, errInteractiveStartRequiresContinuousConfig): + return fmt.Errorf("cannot start %q in interactive mode because it is configured for continuous mode. Call start_continuous instead", workspace.Handle) + default: + return err + } +} + +func humanizeStartContinuousError(workspace *externalRepositoryInfo, err error) error { + switch { + case errors.Is(err, errRootWorkspaceCannotStart): + return fmt.Errorf("cannot start %q in continuous mode because it is a root repository. Choose a standalone repository or a fork workspace instead", workspace.Handle) + case errors.Is(err, errContinuousModeNotConfigured): + return fmt.Errorf("cannot start %q because continuous mode is not configured. Call start_interactive or start_self_drive instead", workspace.Handle) + default: + return err + } +} + +func humanizeGetPendingQuestionError(workspace *externalRepositoryInfo, err error) error { + if errors.Is(err, errNoPendingQuestion) { + return fmt.Errorf("workspace %q has no pending question. Call list_pending_questions to find a workspace that currently needs input", workspace.Handle) + } + return err +} + +func humanizeAnswerPendingQuestionError(workspace *externalRepositoryInfo, err error) error { + switch { + case errors.Is(err, errNoPendingQuestion): + return fmt.Errorf("workspace %q has no pending question. Call list_pending_questions to find a workspace that currently needs input", workspace.Handle) + case errors.Is(err, errPromptTokenRequired): + return fmt.Errorf("pending question answer for %q requires the current prompt token. Call get_pending_question and use the returned promptToken", workspace.Handle) + case errors.Is(err, errQuestionNotAvailable): + return fmt.Errorf("question is no longer current for %q. Call get_pending_question to fetch the latest prompt token and answer the current question", workspace.Handle) + case errors.Is(err, errResponseCannotBeEmpty): + return fmt.Errorf("pending question answer for %q is incomplete: answer text or selected choices are required. Call get_pending_question and provide answer text, selected choices, or both", workspace.Handle) + default: + return err + } +} + +func humanizeSteerNextTurnError(workspace *externalRepositoryInfo, err error) error { + if errors.Is(err, errSteerMessageEmpty) { + return fmt.Errorf("re-steering instruction for %q cannot be empty. Provide a non-empty message", workspace.Handle) + } + return err +} + func humanizeResetWorkspaceError(workspace *externalRepositoryInfo, err error) error { if errors.Is(err, errSessionResetWhileRunning) { return fmt.Errorf("cannot reset %q while it is running. Stop the workspace first", workspace.Handle) @@ -473,6 +952,8 @@ func humanizeForkRepositoryError(workspace *externalRepositoryInfo, err error) e return fmt.Errorf("cannot fork %q because it is already a fork workspace. Choose a standalone repository or a root repository from factory_info instead", workspace.Handle) case errors.Is(err, errGoalContentEmpty): return fmt.Errorf("cannot fork %q without GOAL.md content for the new fork", workspace.Handle) + case errors.Is(err, errForkTitleRequired): + return fmt.Errorf("cannot fork %q because title is required for the new fork", workspace.Handle) default: return err } @@ -510,16 +991,64 @@ func externalBaseURL(r *http.Request) string { } func externalGoalEditURL(baseURL, workspaceName string) (string, error) { + goalURL, errGoalURL := externalWorkspaceURL(baseURL, workspaceName, "goal/edit") + if errGoalURL != nil { + return "", fmt.Errorf("building goal edit link: %w", errGoalURL) + } + return goalURL, nil +} + +func externalWorkspaceURL(baseURL, workspaceName, subpath string) (string, error) { parsedURL, errParse := url.Parse(baseURL) if errParse != nil { - return "", fmt.Errorf("building goal edit link: %w", errParse) + return "", fmt.Errorf("parse base url: %w", errParse) + } + pathSegments := []string{"workspaces", workspaceName} + escapedPathSegments := []string{"workspaces", url.PathEscape(workspaceName)} + trimmedSubpath := strings.Trim(subpath, "/") + if trimmedSubpath != "" { + for segment := range strings.SplitSeq(trimmedSubpath, "/") { + pathSegments = append(pathSegments, segment) + escapedPathSegments = append(escapedPathSegments, url.PathEscape(segment)) + } } - parsedURL.Path = "/workspaces/" + workspaceName + "/goal/edit" - parsedURL.RawPath = "/workspaces/" + url.PathEscape(workspaceName) + "/goal/edit" + parsedURL.Path = "/" + strings.Join(pathSegments, "/") + parsedURL.RawPath = "/" + strings.Join(escapedPathSegments, "/") parsedURL.RawQuery = "" return parsedURL.String(), nil } +func (c *externalMCPContext) workflowLinks(workspace *externalRepositoryInfo, workspacePath string) (externalWorkflowLinks, error) { + workspaceName, errWorkspaceName := c.workflowLinkWorkspaceName(workspace, "workflow links") + if errWorkspaceName != nil { + return externalWorkflowLinks{}, errWorkspaceName + } + baseURL := externalBaseURL(c.request) + progressURL, errProgressURL := externalWorkspaceURL(baseURL, workspaceName, "progress") + if errProgressURL != nil { + return externalWorkflowLinks{}, fmt.Errorf("building workflow links: progress link: %w", errProgressURL) + } + goalEditURL, errGoalEditURL := externalWorkspaceURL(baseURL, workspaceName, "goal/edit") + if errGoalEditURL != nil { + return externalWorkflowLinks{}, fmt.Errorf("building workflow links: goal edit link: %w", errGoalEditURL) + } + + links := externalWorkflowLinks{ + Progress: progressURL, + GoalEdit: goalEditURL, + Respond: "", + } + if !c.server.workflowStateSnapshot(workspacePath).NeedsHumanInput() { + return links, nil + } + respondURL, errRespondURL := externalWorkspaceURL(baseURL, workspaceName, "respond") + if errRespondURL != nil { + return externalWorkflowLinks{}, fmt.Errorf("building workflow links: respond link: %w", errRespondURL) + } + links.Respond = respondURL + return links, nil +} + func resolvedExternalHost(host string, r *http.Request) string { hostName, port := splitExternalHostPort(host) if hostName == "" { diff --git a/cmd/sgai/mcp_external_test.go b/cmd/sgai/mcp_external_test.go index cf00ec1..a5ed342 100644 --- a/cmd/sgai/mcp_external_test.go +++ b/cmd/sgai/mcp_external_test.go @@ -8,7 +8,9 @@ import ( "os/exec" "path/filepath" "slices" + "strings" "testing" + "time" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" @@ -17,15 +19,16 @@ import ( ) type externalRepositoryForTest struct { - Handle string `json:"handle"` - DirectoryName string `json:"directoryName"` - Label string `json:"label"` - Title string `json:"title"` - Path string `json:"path"` - Mode string `json:"mode"` - RootHandle string `json:"rootHandle,omitempty"` - RootPath string `json:"rootPath,omitempty"` - ForkCount int `json:"forkCount,omitempty"` + Handle string `json:"handle"` + DirectoryName string `json:"directoryName"` + Label string `json:"label"` + Title string `json:"title"` + Path string `json:"path"` + Mode string `json:"mode"` + RootHandle string `json:"rootHandle,omitempty"` + RootPath string `json:"rootPath,omitempty"` + ForkCount int `json:"forkCount,omitempty"` + ActiveAgents []string `json:"activeAgents"` } type externalFactoryInfoForTest struct { @@ -40,6 +43,7 @@ type externalSessionActionForTest struct { Running bool `json:"running"` Message string `json:"message"` AlreadyRunning bool `json:"alreadyRunning,omitempty"` + RunningMode string `json:"runningMode,omitempty"` } type externalGoalEditLinkForTest struct { @@ -60,6 +64,108 @@ type externalForkResultForTest struct { Message string `json:"message"` } +type externalWorkspaceArgsForTest struct { + Workspace string `json:"workspace"` +} + +type externalPendingQuestionListForTest struct { + Workspaces []externalRepositoryForTest `json:"workspaces"` +} + +type externalQuestionItemForTest struct { + Question string `json:"question"` + Choices []string `json:"choices"` + MultiSelect bool `json:"multiSelect"` +} + +type externalPendingQuestionForTest struct { + PromptToken string `json:"promptToken"` + Type string `json:"type"` + AgentName string `json:"agentName"` + Message string `json:"message"` + Questions []externalQuestionItemForTest `json:"questions,omitempty"` +} + +type externalPendingQuestionResultForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + PendingQuestion externalPendingQuestionForTest `json:"pendingQuestion"` +} + +type externalAnswerPendingQuestionArgsForTest struct { + Workspace string `json:"workspace"` + PromptToken string `json:"promptToken"` + Answer string `json:"answer"` + SelectedChoices []string `json:"selectedChoices"` +} + +type externalAnswerPendingQuestionResultForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + Success bool `json:"success"` + Message string `json:"message"` +} + +type externalSteerNextTurnArgsForTest struct { + Workspace string `json:"workspace"` + Message string `json:"message"` +} + +type externalSteerNextTurnResultForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + Success bool `json:"success"` + Message string `json:"message"` +} + +type externalWorkflowLinksForTest struct { + Progress string `json:"progress"` + GoalEdit string `json:"goalEdit"` + Respond string `json:"respond,omitempty"` +} + +type externalWorkflowLinksResultForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + Links externalWorkflowLinksForTest +} + +type externalLogEntryForTest struct { + Prefix string `json:"prefix"` + Text string `json:"text"` +} + +type externalWorkspaceLogResultForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + Log []externalLogEntryForTest `json:"log"` +} + +type externalMessageForTest struct { + ID int `json:"id"` + FromAgent string `json:"fromAgent"` + ToAgent string `json:"toAgent"` + Body string `json:"body"` + Read bool `json:"read"` + ReadAt string `json:"readAt,omitempty"` + ReadBy string `json:"readBy,omitempty"` + CreatedAt string `json:"createdAt,omitempty"` +} + +type externalTodoForTest struct { + ID string `json:"id"` + Content string `json:"content"` + Status string `json:"status"` + Priority string `json:"priority"` +} + +type externalAgentTodoSectionForTest struct { + Agent string `json:"agent"` + Todos []externalTodoForTest `json:"todos"` +} + +type externalWorkspaceMessagesAndTodosForTest struct { + Workspace externalRepositoryForTest `json:"workspace"` + Messages []externalMessageForTest `json:"messages"` + ProjectTodos []externalTodoForTest `json:"projectTodos"` + ActiveAgentTodoSections []externalAgentTodoSectionForTest `json:"activeAgentTodoSections"` +} + func connectExternalMCPClient(t *testing.T, server *Server, r *http.Request) *mcp.ClientSession { t.Helper() @@ -127,6 +233,23 @@ func findRepositoryByName(t *testing.T, repositories []externalRepositoryForTest return repository } +func findRepositoryMapByPath(t *testing.T, repositories []any, workspacePath string) map[string]any { + t.Helper() + + for _, item := range repositories { + repository, ok := item.(map[string]any) + require.True(t, ok) + path, ok := repository["path"].(string) + require.True(t, ok) + if path == workspacePath { + return repository + } + } + + require.FailNowf(t, "repository not found", "repository %q not found", workspacePath) + return nil +} + func decodeExternalStructuredContentMap(t *testing.T, result *mcp.CallToolResult) map[string]any { t.Helper() @@ -155,13 +278,27 @@ func TestBuildExternalMCPServerExposesFirstWaveTools(t *testing.T) { require.NoError(t, errList) toolNames := mcpToolNames(result.Tools) - assert.True(t, slices.Contains(toolNames, "factory_info")) - assert.True(t, slices.Contains(toolNames, "start_self_drive")) - assert.True(t, slices.Contains(toolNames, "stop_workspace")) - assert.True(t, slices.Contains(toolNames, "reset_workspace")) - assert.True(t, slices.Contains(toolNames, "goal_edit_link")) - assert.True(t, slices.Contains(toolNames, "fork_repository")) - assert.True(t, slices.Contains(toolNames, "attach_repository")) + slices.Sort(toolNames) + expectedToolNames := []string{ + "answer_pending_question", + "attach_repository", + "factory_info", + "fork_repository", + "get_pending_question", + "goal_edit_link", + "list_pending_questions", + "peek_workspace_log", + "reset_workspace", + "start_continuous", + "start_interactive", + "start_self_drive", + "steer_next_turn", + "stop_workspace", + "workflow_links", + "workspace_messages_and_todos", + } + assert.Equal(t, expectedToolNames, toolNames) + assert.False(t, slices.Contains(toolNames, "inspect_workspace")) assert.False(t, slices.Contains(toolNames, "hello world")) } @@ -193,22 +330,22 @@ func TestExternalMCPFactoryInfoReturnsHybridRepositoryIdentity(t *testing.T) { assert.Equal(t, rootDir, info.StartDirectory) standalone := findRepositoryByName(t, info.Repositories, "solo-dir") - assert.Equal(t, "Solo Title [solo-dir]", standalone.Handle) + assert.Equal(t, "Solo Title", standalone.Handle) assert.Equal(t, "Solo Title", standalone.Label) assert.Equal(t, "Solo Title", standalone.Title) assert.Equal(t, standaloneDir, standalone.Path) assert.Equal(t, "standalone", standalone.Mode) rootRepository := findRepositoryByName(t, info.Repositories, "root-dir") - assert.Equal(t, "Root Title [root-dir]", rootRepository.Handle) - assert.Equal(t, "Root Title", rootRepository.Label) + assert.Equal(t, "root-dir", rootRepository.Handle) + assert.Equal(t, "root-dir", rootRepository.Label) assert.Equal(t, resolveSymlinks(rootWorkspaceDir), rootRepository.Path) assert.Equal(t, "root", rootRepository.Mode) assert.Equal(t, 1, rootRepository.ForkCount) forkRepository := findRepositoryByName(t, info.Repositories, "fork-dir") - assert.Equal(t, "Root Title/Fork Title [fork-dir]", forkRepository.Handle) - assert.Equal(t, "Root Title/Fork Title", forkRepository.Label) + assert.Equal(t, "root-dir/Fork Title", forkRepository.Handle) + assert.Equal(t, "root-dir/Fork Title", forkRepository.Label) assert.Equal(t, resolveSymlinks(forkWorkspaceDir), forkRepository.Path) assert.Equal(t, "fork", forkRepository.Mode) assert.Equal(t, rootRepository.Handle, forkRepository.RootHandle) @@ -233,6 +370,7 @@ func TestExternalMCPFactoryInfoOmitsRoutedNameField(t *testing.T) { repository, ok := repositories[0].(map[string]any) require.True(t, ok) assert.NotContains(t, repository, "routedName") + assert.NotContains(t, repository, "displayLabel") } func TestExternalMCPFactoryInfoPreservesForkModeWithoutAttachedRoot(t *testing.T) { @@ -272,7 +410,7 @@ func TestExternalMCPGoalEditLinkUsesBasenameOnlyPathWithoutWorkspaceDir(t *testi link := decodeExternalStructuredContent[externalGoalEditLinkForTest](t, result) assert.Equal(t, "https://10.10.0.7:9443/workspaces/goal-ws/goal/edit", link.URL) assert.NotContains(t, link.URL, "workspaceDir=") - assert.Equal(t, "Goal Title [goal-ws]", link.Workspace.Handle) + assert.Equal(t, "Goal Title", link.Workspace.Handle) assert.Equal(t, workspaceDir, link.Workspace.Path) } @@ -354,6 +492,112 @@ func TestExternalMCPFactoryInfoKeepsBasenameForDuplicateBasenames(t *testing.T) assert.Equal(t, "shared-ws", secondRepository.DirectoryName) } +func TestExternalMCPFactoryInfoUsesDashboardDisambiguationAndNormalizedActiveAgents(t *testing.T) { + server, _ := setupTestServer(t) + firstDir := filepath.Join(t.TempDir(), "first", "shared-ws") + secondDir := filepath.Join(t.TempDir(), "second", "shared-ws") + activeDir := filepath.Join(t.TempDir(), "active-parent", "active-ws") + require.NoError(t, os.MkdirAll(filepath.Join(firstDir, ".sgai"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(secondDir, ".sgai"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(activeDir, ".sgai"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(firstDir, "GOAL.md"), []byte("---\ntitle: Shared Title\n---\n# First"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(secondDir, "GOAL.md"), []byte("---\ntitle: Shared Title\n---\n# Second"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(activeDir, "GOAL.md"), []byte("---\ntitle: Active Title\n---\n# Active"), 0o644)) + + server.mu.Lock() + server.externalDirs[resolveSymlinks(firstDir)] = true + server.externalDirs[resolveSymlinks(secondDir)] = true + server.externalDirs[resolveSymlinks(activeDir)] = true + server.mu.Unlock() + server.invalidateWorkspaceScanCache() + + attachRunningSessionCoordinator(t, server, resolveSymlinks(activeDir), workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer, react-developer" + workflow.InteractionMode = state.ModeBrainstorming + })) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "factory_info", struct{}{}) + require.False(t, result.IsError) + + info := decodeExternalStructuredContent[externalFactoryInfoForTest](t, result) + payload := decodeExternalStructuredContentMap(t, result) + repositories, ok := payload["repositories"].([]any) + require.True(t, ok) + var firstRepository externalRepositoryForTest + var secondRepository externalRepositoryForTest + activeRepository := findRepositoryByName(t, info.Repositories, "active-ws") + for _, repository := range info.Repositories { + switch repository.Path { + case resolveSymlinks(firstDir): + firstRepository = repository + case resolveSymlinks(secondDir): + secondRepository = repository + } + } + + assert.Equal(t, "Shared Title · first", firstRepository.Handle) + assert.Equal(t, "Shared Title · first", firstRepository.Label) + assert.Equal(t, "Shared Title · second", secondRepository.Handle) + assert.Equal(t, "Shared Title · second", secondRepository.Label) + assert.Equal(t, []string{"go-developer", "react-developer"}, activeRepository.ActiveAgents) + + firstRepositoryMap := findRepositoryMapByPath(t, repositories, resolveSymlinks(firstDir)) + secondRepositoryMap := findRepositoryMapByPath(t, repositories, resolveSymlinks(secondDir)) + activeRepositoryMap := findRepositoryMapByPath(t, repositories, resolveSymlinks(activeDir)) + assert.NotContains(t, firstRepositoryMap, "displayLabel") + assert.NotContains(t, secondRepositoryMap, "displayLabel") + firstActiveAgents, ok := firstRepositoryMap["activeAgents"].([]any) + require.True(t, ok) + assert.Empty(t, firstActiveAgents) + secondActiveAgents, ok := secondRepositoryMap["activeAgents"].([]any) + require.True(t, ok) + assert.Empty(t, secondActiveAgents) + activeAgents, ok := activeRepositoryMap["activeAgents"].([]any) + require.True(t, ok) + assert.Len(t, activeAgents, 2) +} + +func TestExternalMCPStopWorkspaceAcceptsAuthoritativeDashboardLabel(t *testing.T) { + server, _ := setupTestServer(t) + firstDir := filepath.Join(t.TempDir(), "first", "shared-ws") + secondDir := filepath.Join(t.TempDir(), "second", "shared-ws") + require.NoError(t, os.MkdirAll(filepath.Join(firstDir, ".sgai"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(secondDir, ".sgai"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(firstDir, "GOAL.md"), []byte("---\ntitle: Shared Title\n---\n# First"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(secondDir, "GOAL.md"), []byte("---\ntitle: Shared Title\n---\n# Second"), 0o644)) + + server.mu.Lock() + server.externalDirs[resolveSymlinks(firstDir)] = true + server.externalDirs[resolveSymlinks(secondDir)] = true + server.mu.Unlock() + server.invalidateWorkspaceScanCache() + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + infoResult := callExternalTool(t, cs, "factory_info", struct{}{}) + require.False(t, infoResult.IsError) + + info := decodeExternalStructuredContent[externalFactoryInfoForTest](t, infoResult) + var firstRepository externalRepositoryForTest + for _, repository := range info.Repositories { + if repository.Path == resolveSymlinks(firstDir) { + firstRepository = repository + break + } + } + require.Equal(t, "Shared Title · first", firstRepository.Label) + + result := callExternalTool(t, cs, "stop_workspace", externalTargetArgs{Target: firstRepository.Label}) + require.False(t, result.IsError) + + action := decodeExternalStructuredContent[externalSessionActionForTest](t, result) + assert.Equal(t, resolveSymlinks(firstDir), action.Workspace.Path) + assert.Equal(t, "Shared Title · first", action.Workspace.Label) +} + func TestExternalMCPGoalEditLinkRejectsAmbiguousAbsolutePathTarget(t *testing.T) { server, _ := setupTestServer(t) firstDir := filepath.Join(t.TempDir(), "first", "shared-ws") @@ -387,11 +631,11 @@ func TestExternalMCPFormerRootBecomesStandaloneAfterLastForkRemoved(t *testing.T server.classifyCache.delete(workspaceDir) server.classifyCache.delete(resolveSymlinks(workspaceDir)) - server.mu.Lock() - runningSession := new(session) - runningSession.running = true - server.sessions[workspaceDir] = runningSession - server.mu.Unlock() + attachRunningSessionCoordinator(t, server, workspaceDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + workflow.InteractionMode = state.ModeBrainstorming + })) req := httptestNewRequest(t, "http://factory.test/mcp/external") cs := connectExternalMCPClient(t, server, req) @@ -407,18 +651,18 @@ func TestExternalMCPFormerRootBecomesStandaloneAfterLastForkRemoved(t *testing.T action := decodeExternalStructuredContent[externalSessionActionForTest](t, startResult) assert.True(t, action.AlreadyRunning) assert.Equal(t, "running", action.Status) - assert.Equal(t, "session already running", action.Message) + assert.Equal(t, "interactive", action.RunningMode) + assert.Equal(t, "workspace already running in interactive mode", action.Message) } func TestExternalMCPStartSelfDriveReportsAlreadyRunningSession(t *testing.T) { server, rootDir := setupTestServer(t) workspaceDir := setupTestWorkspace(t, server, rootDir, "run-ws") - - server.mu.Lock() - runningSession := new(session) - runningSession.running = true - server.sessions[workspaceDir] = runningSession - server.mu.Unlock() + attachRunningSessionCoordinator(t, server, workspaceDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + workflow.InteractionMode = state.ModeBrainstorming + })) req := httptestNewRequest(t, "http://factory.test/mcp/external") cs := connectExternalMCPClient(t, server, req) @@ -429,7 +673,8 @@ func TestExternalMCPStartSelfDriveReportsAlreadyRunningSession(t *testing.T) { assert.True(t, action.AlreadyRunning) assert.True(t, action.Running) assert.Equal(t, "running", action.Status) - assert.Equal(t, "session already running", action.Message) + assert.Equal(t, "interactive", action.RunningMode) + assert.Equal(t, "workspace already running in interactive mode", action.Message) assert.Equal(t, "run-ws", action.Workspace.DirectoryName) } @@ -492,12 +737,43 @@ func TestExternalMCPForkRepositoryRejectsForkTargetsWithGuidance(t *testing.T) { req := httptestNewRequest(t, "http://factory.test/mcp/external") cs := connectExternalMCPClient(t, server, req) - result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: forkWorkspaceDir, GoalContent: "---\nflow: |\n \"a\" -> \"b\"\n---\n# New Goal"}) + result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: forkWorkspaceDir, GoalContent: "---\nflow: |\n \"a\" -> \"b\"\n---\n# New Goal", Title: "Fork Attempt"}) assert.True(t, result.IsError) assert.Contains(t, externalToolText(t, result), "already a fork workspace") assert.Contains(t, externalToolText(t, result), "standalone repository or a root repository") } +func TestExternalMCPForkRepositoryRequiresTitle(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-title-required") + require.NoError(t, initializeWorkspace(workspaceDir)) + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ntitle: Root Goal\nflow: |\n \"a\" -> \"b\"\n---\n# Goal"), 0o644)) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: workspaceDir, GoalContent: "# Fork Goal", Title: ""}) + assert.True(t, result.IsError) + assert.Contains(t, externalToolText(t, result), "title") + assert.Contains(t, externalToolText(t, result), "required") +} + +func TestExternalMCPForkRepositorySchemaRequiresTitleField(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-title-schema") + require.NoError(t, initializeWorkspace(workspaceDir)) + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ntitle: Root Goal\nflow: |\n \"a\" -> \"b\"\n---\n# Goal"), 0o644)) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + params := new(mcp.CallToolParams) + params.Name = "fork_repository" + params.Arguments = map[string]any{"target": workspaceDir, "goalContent": "# Fork Goal"} + _, errCall := cs.CallTool(context.Background(), params) + require.Error(t, errCall) + assert.Contains(t, errCall.Error(), "missing properties") + assert.Contains(t, errCall.Error(), "title") +} + func TestExternalMCPForkRepositoryCreatesSiblingWorkspace(t *testing.T) { server, rootDir := setupTestServer(t) workspaceDir := setupTestWorkspace(t, server, rootDir, "forkable-ws") @@ -506,7 +782,7 @@ func TestExternalMCPForkRepositoryCreatesSiblingWorkspace(t *testing.T) { req := httptestNewRequest(t, "http://factory.test/mcp/external") cs := connectExternalMCPClient(t, server, req) - result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: workspaceDir, GoalContent: "---\nflow: |\n \"a\" -> \"b\"\n---\n# Fork Goal"}) + result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: workspaceDir, GoalContent: "---\nflow: |\n \"a\" -> \"b\"\n---\n# Fork Goal", Title: "Forkable Copy"}) require.False(t, result.IsError) forkResult := decodeExternalStructuredContent[externalForkResultForTest](t, result) @@ -517,6 +793,57 @@ func TestExternalMCPForkRepositoryCreatesSiblingWorkspace(t *testing.T) { assert.NotEmpty(t, forkResult.CreatedAt) } +func TestExternalMCPForkRepositoryOverridesQuotedCopiedTitleAndPreservesSubmittedBodyText(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-title-override") + require.NoError(t, initializeWorkspace(workspaceDir)) + + rootGoalContent := strings.Join([]string{ + "---", + "\"title\": Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte(rootGoalContent), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "", + " \t", + "---", + "title: Ignored Submitted Title", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + "# Fork Goal", + "", + "Implement the override.", + }, "\n") + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "fork_repository", externalForkArgs{Target: workspaceDir, GoalContent: submittedGoalContent, Title: "External Fork Title"}) + require.False(t, result.IsError) + + forkResult := decodeExternalStructuredContent[externalForkResultForTest](t, result) + forkGoalContent, errRead := os.ReadFile(filepath.Join(forkResult.Workspace.Path, "GOAL.md")) + require.NoError(t, errRead) + + metadata, errParse := parseYAMLFrontmatter(forkGoalContent) + require.NoError(t, errParse) + assert.Equal(t, "External Fork Title", metadata.Title) + frontmatter := forkGoalFrontmatterForTest(t, forkGoalContent) + assert.Contains(t, frontmatter, "flow: |") + assert.Contains(t, frontmatter, "coordinator: root-model") + assert.Equal(t, 1, strings.Count(frontmatter, "title:")+strings.Count(frontmatter, "\"title\":")) + assert.Equal(t, submittedGoalContent, forkGoalBodyForTest(t, forkGoalContent)) +} + func TestExternalMCPAttachRepositoryRequiresAbsolutePath(t *testing.T) { server, _ := setupTestServer(t) req := httptestNewRequest(t, "http://factory.test/mcp/external") @@ -540,10 +867,452 @@ func TestExternalMCPAttachRepositoryAttachesWorkspace(t *testing.T) { attachResult := decodeExternalStructuredContent[externalAttachResultForTest](t, result) assert.True(t, attachResult.HasGoal) assert.Equal(t, resolveSymlinks(externalDir), attachResult.Workspace.Path) - assert.Equal(t, "Attached Goal [attach-me]", attachResult.Workspace.Handle) + assert.Equal(t, "Attached Goal", attachResult.Workspace.Handle) assert.Equal(t, "standalone", attachResult.Workspace.Mode) } +func TestExternalMCPStartInteractiveRejectsContinuousConfiguredWorkspace(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "continuous-ws") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ncontinuousModePrompt: Keep going\nflow: |\n \"a\" -> \"b\"\n---\n# Goal"), 0o644)) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "start_interactive", externalWorkspaceArgsForTest{Workspace: "continuous-ws"}) + assert.True(t, result.IsError) + assert.Contains(t, externalToolText(t, result), "start_continuous") + assert.Contains(t, externalToolText(t, result), "continuous mode") +} + +func TestExternalMCPStartSelfDriveUsesExplicitSelfDriveModeForContinuousConfiguredWorkspace(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "self-drive-continuous-ws") + require.NoError(t, initializeWorkspace(workspaceDir)) + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ncontinuousModePrompt: Keep going\nflow: |\n \"coordinator\"\n---\n# Goal"), 0o644)) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + server.shutdownCtx = ctx + t.Cleanup(func() { + server.stopSession(workspaceDir) + require.Eventually(t, func() bool { + if errRemove := os.RemoveAll(workspaceDir); errRemove != nil { + return false + } + _, errStat := os.Stat(workspaceDir) + return os.IsNotExist(errStat) + }, time.Second, 10*time.Millisecond) + }) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "start_self_drive", externalTargetArgs{Target: "self-drive-continuous-ws"}) + require.False(t, result.IsError) + + action := decodeExternalStructuredContent[externalSessionActionForTest](t, result) + assert.True(t, action.Running) + assert.False(t, action.AlreadyRunning) + assert.Equal(t, "self-drive", action.RunningMode) + assert.Equal(t, "workspace started in self-drive mode", action.Message) + assert.Equal(t, state.ModeSelfDrive, workflowStateFromDisk(t, workspaceDir).InteractionMode) +} + +func TestExternalMCPStartContinuousRejectsWorkspaceWithoutContinuousMode(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "interactive-ws") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\nflow: |\n \"a\" -> \"b\"\n---\n# Goal"), 0o644)) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "start_continuous", externalWorkspaceArgsForTest{Workspace: "interactive-ws"}) + assert.True(t, result.IsError) + assert.Contains(t, externalToolText(t, result), "start_interactive") + assert.Contains(t, externalToolText(t, result), "start_self_drive") + assert.Contains(t, externalToolText(t, result), "continuous mode is not configured") +} + +func TestExternalMCPStartContinuousReportsActualRunningMode(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "already-running-ws") + attachRunningSessionCoordinator(t, server, workspaceDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.InteractionMode = state.ModeSelfDrive + workflow.CurrentAgent = "coordinator" + })) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "start_continuous", externalWorkspaceArgsForTest{Workspace: "already-running-ws"}) + require.False(t, result.IsError) + + action := decodeExternalStructuredContent[externalSessionActionForTest](t, result) + assert.True(t, action.AlreadyRunning) + assert.Equal(t, "self-drive", action.RunningMode) + assert.Equal(t, "workspace already running in self-drive mode", action.Message) +} + +func TestExternalMCPListPendingQuestionsReturnsOnlyLivePendingWorkspaces(t *testing.T) { + server, rootDir := setupTestServer(t) + pendingDir := setupTestWorkspace(t, server, rootDir, "pending-ws") + staleDir := setupTestWorkspace(t, server, rootDir, "stale-ws") + quietDir := setupTestWorkspace(t, server, rootDir, "quiet-ws") + + _, pendingErrCh, pendingCancel := startWaitingSessionQuestion(t, server, pendingDir, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick one" + item.Choices = []string{"A", "B"} + })} + }), "Pick one") + defer func() { + pendingCancel() + require.Error(t, <-pendingErrCh) + }() + + writeWorkflowStateToDisk(t, staleDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.HumanMessage = "stale question" + workflow.MultiChoiceQuestion = multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Old question" + item.Choices = []string{"X", "Y"} + })} + }) + })) + attachRunningSessionCoordinator(t, server, quietDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + })) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "list_pending_questions", struct{}{}) + require.False(t, result.IsError) + + list := decodeExternalStructuredContent[externalPendingQuestionListForTest](t, result) + require.Len(t, list.Workspaces, 1) + assert.Equal(t, "pending-ws", list.Workspaces[0].DirectoryName) +} + +func TestExternalMCPGetPendingQuestionReturnsFullPayload(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "pending-payload-ws") + coord, errCh, cancel := startWaitingSessionQuestion(t, server, workspaceDir, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{ + questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick one" + item.Choices = []string{"A", "B"} + }), + questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick many" + item.Choices = []string{"X", "Y"} + item.MultiSelect = true + }), + } + }), "Please answer both questions") + defer func() { + cancel() + require.Error(t, <-errCh) + }() + promptToken := waitForSessionPromptToken(t, coord) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "get_pending_question", externalWorkspaceArgsForTest{Workspace: "pending-payload-ws"}) + require.False(t, result.IsError) + + payload := decodeExternalStructuredContent[externalPendingQuestionResultForTest](t, result) + assert.Equal(t, "pending-payload-ws", payload.Workspace.DirectoryName) + assert.Equal(t, promptToken, payload.PendingQuestion.PromptToken) + assert.Equal(t, "multi-choice", payload.PendingQuestion.Type) + assert.Equal(t, "coordinator", payload.PendingQuestion.AgentName) + assert.Equal(t, "Please answer both questions", payload.PendingQuestion.Message) + require.Len(t, payload.PendingQuestion.Questions, 2) + assert.Equal(t, "Pick one", payload.PendingQuestion.Questions[0].Question) + assert.Equal(t, []string{"A", "B"}, payload.PendingQuestion.Questions[0].Choices) + assert.True(t, payload.PendingQuestion.Questions[1].MultiSelect) +} + +func TestExternalMCPAnswerPendingQuestionRejectsEmptySubmissionWithResteer(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "empty-answer-ws") + coord, errCh, cancel := startWaitingSessionQuestion(t, server, workspaceDir, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick one" + item.Choices = []string{"A", "B"} + })} + }), "Please explain your choice") + defer func() { + cancel() + require.Error(t, <-errCh) + }() + promptToken := waitForSessionPromptToken(t, coord) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "answer_pending_question", externalAnswerPendingQuestionArgsForTest{ + Workspace: "empty-answer-ws", + PromptToken: promptToken, + Answer: "", + SelectedChoices: nil, + }) + assert.True(t, result.IsError) + message := externalToolText(t, result) + assert.Contains(t, message, "answer text or selected choices are required") + assert.Contains(t, message, "get_pending_question") + assert.Contains(t, message, "provide answer text, selected choices, or both") + assert.NotContains(t, message, "answer every required part of the current question") + assert.True(t, coord.State().NeedsHumanInput()) +} + +func TestExternalMCPAnswerPendingQuestionRejectsStalePromptTokenWithResteer(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "stale-answer-ws") + coord, errCh, cancel := startWaitingSessionQuestion(t, server, workspaceDir, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick one" + item.Choices = []string{"A", "B"} + })} + }), "Please answer") + defer func() { + cancel() + require.Error(t, <-errCh) + }() + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "answer_pending_question", externalAnswerPendingQuestionArgsForTest{ + Workspace: "stale-answer-ws", + PromptToken: "stale-token", + Answer: "I choose A", + SelectedChoices: []string{"A"}, + }) + assert.True(t, result.IsError) + assert.Contains(t, externalToolText(t, result), "question is no longer current") + assert.Contains(t, externalToolText(t, result), "get_pending_question") + assert.True(t, coord.State().NeedsHumanInput()) +} + +func assertExternalPendingQuestionSubmission(t *testing.T, workspaceName, answer string, selectedChoices []string) { + t.Helper() + + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, workspaceName) + coord, errCh, cancel := startWaitingSessionQuestion(t, server, workspaceDir, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Pick one" + item.Choices = []string{"A", "B"} + })} + }), "Please explain your choice") + defer cancel() + promptToken := waitForSessionPromptToken(t, coord) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "answer_pending_question", externalAnswerPendingQuestionArgsForTest{ + Workspace: workspaceName, + PromptToken: promptToken, + Answer: answer, + SelectedChoices: selectedChoices, + }) + require.False(t, result.IsError) + + response := decodeExternalStructuredContent[externalAnswerPendingQuestionResultForTest](t, result) + assert.True(t, response.Success) + assert.Equal(t, "response submitted", response.Message) + assert.Equal(t, workspaceName, response.Workspace.DirectoryName) + require.NoError(t, <-errCh) +} + +func TestExternalMCPAnswerPendingQuestionSubmitsResponse(t *testing.T) { + assertExternalPendingQuestionSubmission(t, "answer-success-ws", "I choose A because it is safer.", []string{"A"}) +} + +func TestExternalMCPAnswerPendingQuestionAllowsChoiceOnlySubmission(t *testing.T) { + assertExternalPendingQuestionSubmission(t, "choice-only-answer-ws", "", []string{"A"}) +} + +func TestExternalMCPAnswerPendingQuestionAllowsTextOnlySubmission(t *testing.T) { + assertExternalPendingQuestionSubmission(t, "text-only-answer-ws", "I prefer A because it is safer.", nil) +} + +func TestExternalMCPSteerNextTurnRejectsEmptyInstruction(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "steer-empty-ws") + _, errCoord := state.NewCoordinatorWith(filepath.Join(workspaceDir, ".sgai", "state.json"), workflowWith(func(*state.Workflow) {})) + require.NoError(t, errCoord) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "steer_next_turn", externalSteerNextTurnArgsForTest{ + Workspace: "steer-empty-ws", + Message: " ", + }) + assert.True(t, result.IsError) + assert.Contains(t, externalToolText(t, result), "cannot be empty") +} + +func TestExternalMCPSteerNextTurnAddsResteeringInstruction(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "steer-success-ws") + attachRunningSessionCoordinator(t, server, workspaceDir, workflowRef(func(*state.Workflow) {})) + coord := server.sessionCoordinator(workspaceDir) + require.NotNil(t, coord) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "steer_next_turn", externalSteerNextTurnArgsForTest{ + Workspace: "steer-success-ws", + Message: "Please branch before changing the API shape.", + }) + require.False(t, result.IsError) + + response := decodeExternalStructuredContent[externalSteerNextTurnResultForTest](t, result) + assert.True(t, response.Success) + assert.Equal(t, "steering instruction added", response.Message) + assert.Equal(t, "steer-success-ws", response.Workspace.DirectoryName) + require.Len(t, coord.State().Messages, 1) + assert.Equal(t, "Human Partner", coord.State().Messages[0].FromAgent) + assert.Equal(t, "coordinator", coord.State().Messages[0].ToAgent) + assert.Equal(t, "Re-steering instruction: Please branch before changing the API shape.", coord.State().Messages[0].Body) +} + +func TestExternalMCPWorkflowLinksReturnsMinimalSetWithoutRespondWhenIdle(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "workflow-links-ws") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ntitle: Workflow Links\n---\n# Goal"), 0o644)) + + req := httptestNewRequest(t, "http://127.0.0.1/mcp/external") + req.Header.Set("X-Forwarded-Proto", "https") + req.Header.Set("X-Forwarded-Host", "10.10.0.7:9443") + + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "workflow_links", externalWorkspaceArgsForTest{Workspace: "workflow-links-ws"}) + require.False(t, result.IsError) + + links := decodeExternalStructuredContent[externalWorkflowLinksResultForTest](t, result) + assert.Equal(t, "https://10.10.0.7:9443/workspaces/workflow-links-ws/progress", links.Links.Progress) + assert.Equal(t, "https://10.10.0.7:9443/workspaces/workflow-links-ws/goal/edit", links.Links.GoalEdit) + assert.Empty(t, links.Links.Respond) + assert.Equal(t, "workflow-links-ws", links.Workspace.DirectoryName) +} + +func TestExternalMCPWorkflowLinksIncludesRespondWhenInputIsPending(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "workflow-pending-ws") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ntitle: Workflow Pending\n---\n# Goal"), 0o644)) + _, errCh, cancel := startWaitingSessionQuestion(t, server, workspaceDir, nil, "Please answer") + defer func() { + cancel() + require.Error(t, <-errCh) + }() + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "workflow_links", externalWorkspaceArgsForTest{Workspace: "workflow-pending-ws"}) + require.False(t, result.IsError) + + links := decodeExternalStructuredContent[externalWorkflowLinksResultForTest](t, result) + assert.Equal(t, "http://factory.test/workspaces/workflow-pending-ws/progress", links.Links.Progress) + assert.Equal(t, "http://factory.test/workspaces/workflow-pending-ws/goal/edit", links.Links.GoalEdit) + assert.Equal(t, "http://factory.test/workspaces/workflow-pending-ws/respond", links.Links.Respond) + assert.Equal(t, "workflow-pending-ws", links.Workspace.DirectoryName) +} + +func TestExternalMCPWorkflowLinksRejectsAmbiguousAbsolutePathTarget(t *testing.T) { + server, _ := setupTestServer(t) + firstDir := filepath.Join(t.TempDir(), "first", "shared-ws") + secondDir := filepath.Join(t.TempDir(), "second", "shared-ws") + require.NoError(t, os.MkdirAll(filepath.Join(firstDir, ".sgai"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(secondDir, ".sgai"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(firstDir, "GOAL.md"), []byte("---\ntitle: First Shared\n---\n# First"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(secondDir, "GOAL.md"), []byte("---\ntitle: Second Shared\n---\n# Second"), 0o644)) + + server.mu.Lock() + server.externalDirs[resolveSymlinks(firstDir)] = true + server.externalDirs[resolveSymlinks(secondDir)] = true + server.mu.Unlock() + server.invalidateWorkspaceScanCache() + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "workflow_links", externalWorkspaceArgsForTest{Workspace: firstDir}) + assert.True(t, result.IsError) + + message := externalToolText(t, result) + assert.Contains(t, message, "workflow links") + assert.Contains(t, message, "basename") + assert.Contains(t, message, "ambiguous") + assert.NotContains(t, message, "goal edit link") +} + +func TestExternalMCPPeekWorkspaceLogReturnsLiveBufferOnly(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "log-ws") + sess := newTestServeSession(nil, true) + sess.outputLog = newCircularLogBuffer() + sess.outputLog.add(logLine{prefix: "stdout", text: "first live line"}) + sess.outputLog.add(logLine{prefix: "stderr", text: "second live line"}) + server.mu.Lock() + server.sessions[workspaceDir] = sess + server.mu.Unlock() + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, ".sgai", "session.log"), []byte("historical line"), 0o644)) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "peek_workspace_log", externalWorkspaceArgsForTest{Workspace: "log-ws"}) + require.False(t, result.IsError) + + logResult := decodeExternalStructuredContent[externalWorkspaceLogResultForTest](t, result) + require.Len(t, logResult.Log, 2) + assert.Equal(t, []externalLogEntryForTest{{Prefix: "stdout", Text: "first live line"}, {Prefix: "stderr", Text: "second live line"}}, logResult.Log) +} + +func TestExternalMCPWorkspaceMessagesAndTodosUsesInternalOrderAndActiveAgentTodos(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "messages-ws") + attachRunningSessionCoordinator(t, server, workspaceDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer, react-developer" + workflow.Messages = []state.Message{ + messageWith(func(message *state.Message) { + message.ID = 1 + message.FromAgent = "coordinator" + message.ToAgent = "go-developer" + message.Body = "first message" + }), + messageWith(func(message *state.Message) { + message.ID = 2 + message.FromAgent = "go-developer" + message.ToAgent = "react-developer" + message.Body = "second message" + message.Read = true + }), + } + workflow.ProjectTodos = []state.TodoItem{{ID: "p1", Content: "project todo", Status: "pending", Priority: "high"}} + workflow.TodosByAgent = map[string][]state.TodoItem{ + "go-developer": {{ID: "g1", Content: "go todo", Status: "pending", Priority: "high"}}, + "react-developer": {{ID: "r1", Content: "react todo", Status: "in_progress", Priority: "medium"}}, + "coordinator": {{ID: "c1", Content: "hidden todo", Status: "pending", Priority: "low"}}, + } + })) + + req := httptestNewRequest(t, "http://factory.test/mcp/external") + cs := connectExternalMCPClient(t, server, req) + result := callExternalTool(t, cs, "workspace_messages_and_todos", externalWorkspaceArgsForTest{Workspace: "messages-ws"}) + require.False(t, result.IsError) + + export := decodeExternalStructuredContent[externalWorkspaceMessagesAndTodosForTest](t, result) + require.Len(t, export.Messages, 2) + assert.Equal(t, 1, export.Messages[0].ID) + assert.Equal(t, 2, export.Messages[1].ID) + require.Len(t, export.ProjectTodos, 1) + assert.Equal(t, "project todo", export.ProjectTodos[0].Content) + require.Len(t, export.ActiveAgentTodoSections, 2) + assert.Equal(t, "go-developer", export.ActiveAgentTodoSections[0].Agent) + assert.Equal(t, "go todo", export.ActiveAgentTodoSections[0].Todos[0].Content) + assert.Equal(t, "react-developer", export.ActiveAgentTodoSections[1].Agent) + assert.Equal(t, "react todo", export.ActiveAgentTodoSections[1].Todos[0].Content) +} + func httptestNewRequest(t *testing.T, rawURL string) *http.Request { t.Helper() diff --git a/cmd/sgai/serve_api_test.go b/cmd/sgai/serve_api_test.go index 1b8a2ec..13e59ab 100644 --- a/cmd/sgai/serve_api_test.go +++ b/cmd/sgai/serve_api_test.go @@ -2147,6 +2147,150 @@ func TestHandleAPIForkWorkspace(t *testing.T) { }) } +func TestHandleAPIForkWorkspaceCopiesRootFrontmatterAndPreservesSubmittedBodyText(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-http") + require.NoError(t, initializeWorkspace(workspaceDir)) + + rootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte(rootGoalContent), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "---", + "title: Edited In Browser", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + "# Fork Goal", + "", + "Implement the HTTP fork.", + }, "\n") + requestBody, errMarshal := json.Marshal(apiForkRequest{GoalContent: submittedGoalContent}) + require.NoError(t, errMarshal) + + w := serveHTTP(server, "POST", "/api/v1/workspaces/fork-http/fork", string(requestBody)) + require.Equal(t, http.StatusCreated, w.Code) + + var response apiForkResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&response)) + forkGoalContent, errRead := os.ReadFile(filepath.Join(response.Dir, "GOAL.md")) + require.NoError(t, errRead) + + expectedGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + }, "\n") + "\n\n" + submittedGoalContent + assert.Equal(t, expectedGoalContent, string(forkGoalContent)) +} + +func TestHandleAPIForkWorkspacePreservesSubmittedBodyTextThatLooksLikeFrontmatter(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-http-leading-whitespace") + require.NoError(t, initializeWorkspace(workspaceDir)) + + rootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte(rootGoalContent), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "", + " \t", + "---", + "title: Edited In Browser", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + "# Fork Goal", + "", + "Implement the HTTP fork.", + }, "\n") + requestBody, errMarshal := json.Marshal(apiForkRequest{GoalContent: submittedGoalContent}) + require.NoError(t, errMarshal) + + w := serveHTTP(server, "POST", "/api/v1/workspaces/fork-http-leading-whitespace/fork", string(requestBody)) + require.Equal(t, http.StatusCreated, w.Code) + + var response apiForkResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&response)) + forkGoalContent, errRead := os.ReadFile(filepath.Join(response.Dir, "GOAL.md")) + require.NoError(t, errRead) + + expectedGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + }, "\n") + "\n\n" + submittedGoalContent + assert.Equal(t, expectedGoalContent, string(forkGoalContent)) +} + +func TestHandleAPIForkWorkspaceAcceptsFrontmatterLookingBodyText(t *testing.T) { + server, rootDir := setupTestServer(t) + workspaceDir := setupTestWorkspace(t, server, rootDir, "fork-http-empty-frontmatter") + require.NoError(t, initializeWorkspace(workspaceDir)) + require.NoError(t, os.WriteFile(filepath.Join(workspaceDir, "GOAL.md"), []byte("---\ntitle: Root Goal\nflow: |\n \"a\" -> \"b\"\n---\n# Goal"), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "", + " \t", + "---", + "title: Edited In Browser", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + " \t", + }, "\n") + requestBody, errMarshal := json.Marshal(apiForkRequest{GoalContent: submittedGoalContent}) + require.NoError(t, errMarshal) + + w := serveHTTP(server, "POST", "/api/v1/workspaces/fork-http-empty-frontmatter/fork", string(requestBody)) + require.Equal(t, http.StatusCreated, w.Code) + + var response apiForkResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&response)) + forkGoalContent, errRead := os.ReadFile(filepath.Join(response.Dir, "GOAL.md")) + require.NoError(t, errRead) + + expectedGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"a\" -> \"b\"", + "---", + }, "\n") + "\n\n" + submittedGoalContent + assert.Equal(t, expectedGoalContent, string(forkGoalContent)) +} + func TestHandleAPIDeleteFork(t *testing.T) { t.Run("missingWorkspace", func(t *testing.T) { server, _ := setupTestServer(t) diff --git a/cmd/sgai/service_session.go b/cmd/sgai/service_session.go index 945b851..619f56c 100644 --- a/cmd/sgai/service_session.go +++ b/cmd/sgai/service_session.go @@ -14,13 +14,23 @@ import ( ) var ( - errRootWorkspaceCannotStart = errors.New("root workspace cannot start agentic work") - errSessionResetWhileRunning = errors.New("cannot reset while session is running") - errNoPendingQuestion = errors.New("no pending question") - errPromptTokenRequired = errors.New("prompt token is required") - errResponseCannotBeEmpty = errors.New("response cannot be empty") - errQuestionNotAvailable = errors.New("question not available") - errSteerMessageEmpty = errors.New("message cannot be empty") + errRootWorkspaceCannotStart = errors.New("root workspace cannot start agentic work") + errSessionResetWhileRunning = errors.New("cannot reset while session is running") + errInteractiveStartRequiresContinuousConfig = errors.New("interactive start requires continuous configuration") + errContinuousModeNotConfigured = errors.New("continuous mode is not configured") + errNoPendingQuestion = errors.New("no pending question") + errPromptTokenRequired = errors.New("prompt token is required") + errResponseCannotBeEmpty = errors.New("response cannot be empty") + errQuestionNotAvailable = errors.New("question not available") + errSteerMessageEmpty = errors.New("message cannot be empty") +) + +type sessionStartMode string + +const ( + sessionStartModeSelfDrive sessionStartMode = "self-drive" + sessionStartModeInteractive sessionStartMode = "interactive" + sessionStartModeContinuous sessionStartMode = "continuous" ) type sessionStartResult struct { @@ -29,41 +39,53 @@ type sessionStartResult struct { Running bool Message string AlreadyRunning bool + RunningMode string } func (s *Server) startSessionService(workspacePath string, auto bool) (sessionStartResult, error) { + requestedMode := requestedSessionStartMode(auto, readContinuousModePrompt(workspacePath) != "") + return s.startSessionInModeService(workspacePath, requestedMode) +} + +func requestedSessionStartMode(auto, continuousConfigured bool) sessionStartMode { + switch { + case continuousConfigured: + return sessionStartModeContinuous + case auto: + return sessionStartModeSelfDrive + default: + return sessionStartModeInteractive + } +} + +func (s *Server) startSessionInModeService(workspacePath string, requestedMode sessionStartMode) (sessionStartResult, error) { if s.classifyWorkspaceCached(workspacePath) == workspaceRoot { return sessionStartResult{}, errRootWorkspaceCannotStart } name := filepath.Base(workspacePath) - if s.sessionRunning(workspacePath) { + if runningMode, okRunningMode := s.runningSessionMode(workspacePath); okRunningMode { return sessionStartResult{ Name: name, Status: "running", Running: true, Message: "session already running", AlreadyRunning: true, + RunningMode: string(runningMode), }, nil } + if errValidateStartMode := validateStartModeRequest(workspacePath, requestedMode); errValidateStartMode != nil { + return sessionStartResult{}, errValidateStartMode + } + if errValidateStart := validateStartSessionWorkspace(workspacePath); errValidateStart != nil { return sessionStartResult{}, errValidateStart } coord := s.workspaceCoordinator(workspacePath) - continuousPrompt := readContinuousModePrompt(workspacePath) - - var interactionMode string - switch { - case continuousPrompt != "": - interactionMode = state.ModeContinuous - case auto: - interactionMode = state.ModeSelfDrive - default: - interactionMode = state.ModeBrainstorming - } + interactionMode := interactionModeForSessionStart(requestedMode) if errUpdate := coord.UpdateState(func(wf *state.Workflow) { wf.InteractionMode = interactionMode @@ -74,12 +96,17 @@ func (s *Server) startSessionService(workspacePath string, auto bool) (sessionSt result := s.startSession(workspacePath) if result.alreadyRunning { + runningMode, okRunningMode := s.runningSessionMode(workspacePath) + if !okRunningMode { + runningMode = requestedMode + } return sessionStartResult{ Name: name, Status: "running", Running: true, Message: "session already running", AlreadyRunning: true, + RunningMode: string(runningMode), }, nil } @@ -93,9 +120,72 @@ func (s *Server) startSessionService(workspacePath string, auto bool) (sessionSt Running: true, Message: "session started", AlreadyRunning: false, + RunningMode: string(requestedMode), }, nil } +func validateStartModeRequest(workspacePath string, requestedMode sessionStartMode) error { + continuousConfigured := readContinuousModePrompt(workspacePath) != "" + switch requestedMode { + case sessionStartModeSelfDrive: + return nil + case sessionStartModeInteractive: + if continuousConfigured { + return errInteractiveStartRequiresContinuousConfig + } + case sessionStartModeContinuous: + if !continuousConfigured { + return errContinuousModeNotConfigured + } + } + return nil +} + +func interactionModeForSessionStart(requestedMode sessionStartMode) string { + switch requestedMode { + case sessionStartModeInteractive: + return state.ModeBrainstorming + case sessionStartModeSelfDrive: + return state.ModeSelfDrive + case sessionStartModeContinuous: + return state.ModeContinuous + default: + return state.ModeBrainstorming + } +} + +func runningModeFromInteractionMode(interactionMode string) sessionStartMode { + switch interactionMode { + case state.ModeSelfDrive: + return sessionStartModeSelfDrive + case state.ModeContinuous: + return sessionStartModeContinuous + default: + return sessionStartModeInteractive + } +} + +func (s *Server) runningSessionMode(workspacePath string) (sessionStartMode, bool) { + s.mu.Lock() + sess := s.sessions[workspacePath] + s.mu.Unlock() + if sess == nil { + return "", false + } + + sess.mu.Lock() + running := sess.running + coord := sess.coord + sess.mu.Unlock() + if !running { + return "", false + } + if coord != nil { + return runningModeFromInteractionMode(coord.State().InteractionMode), true + } + return runningModeFromInteractionMode(s.loadWorkspaceState(workspacePath).InteractionMode), true +} + func (s *Server) sessionRunning(workspacePath string) bool { s.mu.Lock() sess := s.sessions[workspacePath] diff --git a/cmd/sgai/service_workspace.go b/cmd/sgai/service_workspace.go index 91978ba..51abd35 100644 --- a/cmd/sgai/service_workspace.go +++ b/cmd/sgai/service_workspace.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "errors" "fmt" "math/rand/v2" @@ -10,15 +11,17 @@ import ( "slices" "strings" "time" + "unicode" "github.com/ucirello/sgai/pkg/state" ) var ( - errForkOfFork = errors.New("forks cannot create new forks") - errGoalContentEmpty = errors.New("GOAL.md must have content describing the goal") - errDirectoryExists = errors.New("a directory with this name already exists") - errMessageNotFound = errors.New("message not found") + errForkOfFork = errors.New("forks cannot create new forks") + errGoalContentEmpty = errors.New("GOAL.md must have content describing the goal") + errForkTitleRequired = errors.New("fork title is required") + errDirectoryExists = errors.New("a directory with this name already exists") + errMessageNotFound = errors.New("message not found") ) func generateRandomForkName() string { @@ -52,10 +55,23 @@ type forkWorkspaceResult struct { } func (s *Server) forkWorkspaceService(workspacePath, goalContent string) (forkWorkspaceResult, error) { + return s.forkWorkspaceServiceWithOptions(workspacePath, goalContent, forkWorkspaceOptions{title: "", requireTitle: false}) +} + +type forkWorkspaceOptions struct { + title string + requireTitle bool +} + +func (s *Server) forkWorkspaceServiceWithOptions(workspacePath, goalContent string, options forkWorkspaceOptions) (forkWorkspaceResult, error) { if s.classifyWorkspaceCached(workspacePath) == workspaceFork { return forkWorkspaceResult{}, errForkOfFork } + if options.requireTitle && sanitizedPersistedGoalTitle(options.title) == "" { + return forkWorkspaceResult{}, errForkTitleRequired + } + if goalContentBodyIsEmpty(goalContent) { return forkWorkspaceResult{}, errGoalContentEmpty } @@ -87,7 +103,11 @@ func (s *Server) forkWorkspaceService(workspacePath, goalContent string) (forkWo if errExclude := addGitExclude(forkPath); errExclude != nil { return forkWorkspaceResult{}, failForkWorkspaceSetup(workspacePath, forkPath, "failed to add git exclude", errExclude) } - if errGoal := writeGoalContent(forkPath, goalContent); errGoal != nil { + forkGoalContent, errForkGoalContent := s.buildForkGoalContent(workspacePath, goalContent, options) + if errForkGoalContent != nil { + return forkWorkspaceResult{}, failForkWorkspaceSetup(workspacePath, forkPath, "failed to prepare GOAL.md", errForkGoalContent) + } + if errGoal := writeGoalBytes(forkPath, forkGoalContent); errGoal != nil { return forkWorkspaceResult{}, failForkWorkspaceSetup(workspacePath, forkPath, "failed to create GOAL.md", errGoal) } @@ -115,6 +135,107 @@ func (s *Server) forkWorkspaceService(workspacePath, goalContent string) (forkWo }, nil } +func (s *Server) buildForkGoalContent(workspacePath, goalContent string, options forkWorkspaceOptions) ([]byte, error) { + goalPath := filepath.Join(workspacePath, "GOAL.md") + rootGoalContent, errRead := os.ReadFile(goalPath) + if errRead != nil { + return nil, fmt.Errorf("reading parent GOAL.md: %w", errRead) + } + + body := submittedForkGoalBody(goalContent) + sections, hasFrontmatter, errSections := parentGoalFrontmatterSections(rootGoalContent) + if errSections != nil { + return nil, errSections + } + if hasFrontmatter { + if errValidate := validateParentGoalFrontmatter(rootGoalContent); errValidate != nil { + return nil, errValidate + } + } + if !hasFrontmatter { + if sanitizedPersistedGoalTitle(options.title) == "" { + return []byte(body), nil + } + frontmatter, errFrontmatter := frontmatterWithOverriddenTitle(nil, nil, options.title) + if errFrontmatter != nil { + return nil, errFrontmatter + } + return composeForkGoalContent(frontmatter, nil, body), nil + } + + frontmatter := sections.frontmatter + if sanitizedPersistedGoalTitle(options.title) != "" { + var errFrontmatter error + frontmatter, errFrontmatter = frontmatterWithOverriddenTitle(sections.frontmatter, sections.lineEnding, options.title) + if errFrontmatter != nil { + return nil, errFrontmatter + } + } + + return composeForkGoalContent(frontmatter, sections.lineEnding, body), nil +} + +func parentGoalFrontmatterSections(content []byte) (goalFrontmatterSections, bool, error) { + var zeroSections goalFrontmatterSections + trimmedContent := bytes.TrimLeftFunc(content, unicode.IsSpace) + if !bytes.HasPrefix(trimmedContent, []byte("---")) { + return zeroSections, false, nil + } + if !bytes.HasPrefix(content, []byte("---")) { + return zeroSections, false, errors.New("parent GOAL.md frontmatter must start at beginning of file") + } + sections, errSections := splitGoalFrontmatterSections(content) + if errSections != nil { + return zeroSections, false, fmt.Errorf("invalid parent GOAL.md frontmatter: %w", errSections) + } + return sections, true, nil +} + +func validateParentGoalFrontmatter(content []byte) error { + _, errParse := parseYAMLFrontmatter(content) + if errParse != nil { + return fmt.Errorf("invalid parent GOAL.md frontmatter: %w", errParse) + } + return nil +} + +func submittedForkGoalBody(goalContent string) string { + return goalContent +} + +func frontmatterWithOverriddenTitle(frontmatter, lineEnding []byte, title string) ([]byte, error) { + title = sanitizedPersistedGoalTitle(title) + if title == "" { + return frontmatter, nil + } + updatedFrontmatter, errUpdate := updatedGoalFrontmatter(frontmatter, title) + if errUpdate != nil { + return nil, errUpdate + } + return frontmatterWithLineEnding(updatedFrontmatter, normalizedLineEnding(lineEnding)), nil +} + +func normalizedLineEnding(lineEnding []byte) []byte { + if len(lineEnding) == 0 { + return []byte("\n") + } + return lineEnding +} + +func composeForkGoalContent(frontmatter, lineEnding []byte, body string) []byte { + resolvedLineEnding := string(lineEnding) + if resolvedLineEnding == "" { + resolvedLineEnding = "\n" + } + + content := "---" + resolvedLineEnding + string(frontmatter) + if !strings.HasSuffix(content, resolvedLineEnding) { + content += resolvedLineEnding + } + content += "---" + resolvedLineEnding + resolvedLineEnding + body + return []byte(content) +} + func failForkWorkspaceSetup(workspacePath, forkPath, message string, errCause error) error { errSetup := fmt.Errorf("%s: %w", message, errCause) errRollback := rollbackForkWorkspaceCreation(workspacePath, forkPath) @@ -147,13 +268,17 @@ func rollbackForkWorkspaceCreation(workspacePath, forkPath string) error { } func goalContentBodyIsEmpty(goalContent string) bool { - body := stripFrontmatter(goalContent) + body := submittedForkGoalBody(goalContent) return strings.TrimSpace(body) == "" } func writeGoalContent(dir, content string) error { + return writeGoalBytes(dir, []byte(content)) +} + +func writeGoalBytes(dir string, content []byte) error { goalPath := filepath.Join(dir, "GOAL.md") - if errWrite := os.WriteFile(goalPath, []byte(content), 0o644); errWrite != nil { + if errWrite := os.WriteFile(goalPath, content, 0o644); errWrite != nil { return fmt.Errorf("writing GOAL.md: %w", errWrite) } return nil diff --git a/cmd/sgai/service_workspace_test.go b/cmd/sgai/service_workspace_test.go index f456798..f1bff00 100644 --- a/cmd/sgai/service_workspace_test.go +++ b/cmd/sgai/service_workspace_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "errors" "os" "path/filepath" @@ -14,6 +15,31 @@ import ( "github.com/ucirello/sgai/pkg/state" ) +func forkGoalSectionsForTest(t *testing.T, content []byte) goalFrontmatterSections { + t.Helper() + + sections, errSections := splitGoalFrontmatterSections(content) + require.NoError(t, errSections) + return sections +} + +func forkGoalFrontmatterForTest(t *testing.T, content []byte) string { + t.Helper() + + return string(forkGoalSectionsForTest(t, content).frontmatter) +} + +func forkGoalBodyForTest(t *testing.T, content []byte) string { + t.Helper() + + sections := forkGoalSectionsForTest(t, content) + after := sections.after + for range 2 { + after = bytes.TrimPrefix(after, sections.lineEnding) + } + return string(after) +} + func TestForkWorkspaceService(t *testing.T) { tests := []struct { name string @@ -27,7 +53,7 @@ func TestForkWorkspaceService(t *testing.T) { { name: "forkFromRootWorkspace", workspace: "", - goalContent: "---\nflow: |\n \"agent1\" -> \"agent2\"\n---\n# Test Goal", + goalContent: "# Test Goal", setupFunc: func(t *testing.T, rootDir string) string { t.Helper() workspacePath := filepath.Join(rootDir, "root-workspace") @@ -35,7 +61,18 @@ func TestForkWorkspaceService(t *testing.T) { require.NoError(t, initializeWorkspace(workspacePath)) goalPath := filepath.Join(workspacePath, "GOAL.md") - require.NoError(t, os.WriteFile(goalPath, []byte("initial goal"), 0o644)) + rootGoalContent := strings.Join([]string{ + "---", + "title: Root Workspace", + "flow: |", + " \"agent1\" -> \"agent2\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(goalPath, []byte(rootGoalContent), 0o644)) return workspacePath }, @@ -43,7 +80,17 @@ func TestForkWorkspaceService(t *testing.T) { errContains: "", validate: func(t *testing.T, _ string, result forkWorkspaceResult) { t.Helper() - const wantGoalContent = "---\nflow: |\n \"agent1\" -> \"agent2\"\n---\n# Test Goal" + wantGoalContent := strings.Join([]string{ + "---", + "title: Root Workspace", + "flow: |", + " \"agent1\" -> \"agent2\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Test Goal", + }, "\n") assertForkNameHasRootPrefix(t, result.Name, "root-workspace") assert.DirExists(t, result.Dir) @@ -101,9 +148,9 @@ func TestForkWorkspaceService(t *testing.T) { validate: nil, }, { - name: "forkWithOnlyFrontmatter", + name: "forkWithOnlyWhitespaceGoalContent", workspace: "", - goalContent: "---\nflow: |\n \"agent1\" -> \"agent2\"\n---\n", + goalContent: " \n\t", setupFunc: func(t *testing.T, rootDir string) string { t.Helper() workspacePath := filepath.Join(rootDir, "root-workspace") @@ -150,6 +197,155 @@ func TestForkWorkspaceService(t *testing.T) { } } +func TestForkWorkspaceServiceCopiesRootFrontmatterAndPreservesSubmittedBodyText(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + workspacePath := filepath.Join(rootDir, "root-workspace") + require.NoError(t, os.MkdirAll(workspacePath, 0o755)) + require.NoError(t, initializeWorkspace(workspacePath)) + + rootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + "", + "Root body.", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspacePath, "GOAL.md"), []byte(rootGoalContent), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "---", + "title: Edited In Browser", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + "# Fork Task", + "", + "Implement the fork contract.", + }, "\n") + + result, errFork := server.forkWorkspaceService(workspacePath, submittedGoalContent) + require.NoError(t, errFork) + + forkGoalContent, errRead := os.ReadFile(filepath.Join(result.Dir, "GOAL.md")) + require.NoError(t, errRead) + + expectedGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + }, "\n") + "\n\n" + submittedGoalContent + assert.Equal(t, expectedGoalContent, string(forkGoalContent)) +} + +func TestForkWorkspaceServicePreservesSubmittedBodyTextThatLooksLikeFrontmatter(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + workspacePath := filepath.Join(rootDir, "root-workspace") + require.NoError(t, os.MkdirAll(workspacePath, 0o755)) + require.NoError(t, initializeWorkspace(workspacePath)) + + rootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspacePath, "GOAL.md"), []byte(rootGoalContent), 0o644)) + + submittedGoalContent := strings.Join([]string{ + "", + " \t", + "---", + "title: Edited In Browser", + "flow: |", + " \"browser\" -> \"editor\"", + "---", + "", + "# Fork Task", + "", + "Implement the fork contract.", + }, "\n") + + result, errFork := server.forkWorkspaceService(workspacePath, submittedGoalContent) + require.NoError(t, errFork) + + forkGoalContent, errRead := os.ReadFile(filepath.Join(result.Dir, "GOAL.md")) + require.NoError(t, errRead) + + expectedGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "models:", + " coordinator: root-model", + "---", + }, "\n") + "\n\n" + submittedGoalContent + assert.Equal(t, expectedGoalContent, string(forkGoalContent)) +} + +func TestForkWorkspaceServiceRejectsMalformedParentFrontmatter(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + workspacePath := filepath.Join(rootDir, "root-workspace") + require.NoError(t, os.MkdirAll(workspacePath, 0o755)) + require.NoError(t, initializeWorkspace(workspacePath)) + + malformedRootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: |", + " \"coordinator\" -> \"go-developer\"", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspacePath, "GOAL.md"), []byte(malformedRootGoalContent), 0o644)) + + _, errFork := server.forkWorkspaceService(workspacePath, "# Fork Goal") + require.Error(t, errFork) + require.ErrorContains(t, errFork, "failed to prepare GOAL.md") + require.ErrorContains(t, errFork, "invalid parent GOAL.md frontmatter") +} + +func TestForkWorkspaceServiceRejectsParentFrontmatterWithInvalidYAML(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + workspacePath := filepath.Join(rootDir, "root-workspace") + require.NoError(t, os.MkdirAll(workspacePath, 0o755)) + require.NoError(t, initializeWorkspace(workspacePath)) + + invalidRootGoalContent := strings.Join([]string{ + "---", + "title: Root Goal", + "flow: [", + "---", + "", + "# Root Goal", + }, "\n") + require.NoError(t, os.WriteFile(filepath.Join(workspacePath, "GOAL.md"), []byte(invalidRootGoalContent), 0o644)) + + _, errFork := server.forkWorkspaceService(workspacePath, "# Fork Goal") + require.Error(t, errFork) + require.ErrorContains(t, errFork, "failed to prepare GOAL.md") + require.ErrorContains(t, errFork, "invalid parent GOAL.md frontmatter") +} + func TestForkExternalWorkspaceSiblingPlacement(t *testing.T) { sgaiRoot := t.TempDir() externalParent := t.TempDir() @@ -828,7 +1024,7 @@ func TestGoalContentBodyIsEmpty(t *testing.T) { { name: "onlyFrontmatter", content: "---\nflow: |\n \"agent1\" -> \"agent2\"\n---\n", - expected: true, + expected: false, }, { name: "frontmatterWithBody", @@ -838,6 +1034,11 @@ func TestGoalContentBodyIsEmpty(t *testing.T) { { name: "frontmatterWithWhitespaceBody", content: "---\nflow: |\n \"agent1\" -> \"agent2\"\n---\n \n\t\n", + expected: false, + }, + { + name: "onlyWhitespace", + content: " \n\t\n", expected: true, }, { diff --git a/cmd/sgai/webapp/src/App.tsx b/cmd/sgai/webapp/src/App.tsx index 803076b..3dd82dd 100644 --- a/cmd/sgai/webapp/src/App.tsx +++ b/cmd/sgai/webapp/src/App.tsx @@ -6,7 +6,7 @@ import { TooltipProvider } from "./components/ui/tooltip"; import { useNotifications } from "./hooks/useNotifications"; import { useFactoryState } from "./lib/factory-state"; import { getFirstPendingResponseTarget } from "./lib/pending-response"; -import { buildWorkspacePath } from "./lib/workspace-identity"; +import { buildWorkspacePathWithDisambiguator, buildWorkspaceRouteDisambiguators } from "./lib/workspace-identity"; function isEditableTarget(target: EventTarget | null): boolean { if (!(target instanceof Element)) { @@ -22,6 +22,10 @@ function isEditableTarget(target: EventTarget | null): boolean { function PendingResponseShortcut() { const navigate = useNavigate(); const { workspaces } = useFactoryState(); + const workspaceRouteDisambiguators = useMemo( + () => buildWorkspaceRouteDisambiguators(workspaces), + [workspaces], + ); const firstPendingResponseTarget = useMemo( () => getFirstPendingResponseTarget(workspaces), [workspaces], @@ -42,12 +46,12 @@ function PendingResponseShortcut() { } event.preventDefault(); - navigate(buildWorkspacePath(firstPendingResponseTarget, "respond")); + navigate(buildWorkspacePathWithDisambiguator(firstPendingResponseTarget, workspaceRouteDisambiguators, "respond")); }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [firstPendingResponseTarget, navigate]); + }, [firstPendingResponseTarget, navigate, workspaceRouteDisambiguators]); return null; } diff --git a/cmd/sgai/webapp/src/components/ActionBar.tsx b/cmd/sgai/webapp/src/components/ActionBar.tsx index a8eb3af..b4c0368 100644 --- a/cmd/sgai/webapp/src/components/ActionBar.tsx +++ b/cmd/sgai/webapp/src/components/ActionBar.tsx @@ -20,6 +20,7 @@ interface ActionBarProps { isRunning: boolean; onActionClick: (action: ApiActionEntry, variables: Record) => void; actionConfigError?: string; + disabledReason?: string; accessibilityContext?: string; className?: string; buttonClassName?: string; @@ -76,6 +77,7 @@ export function ActionBar({ isRunning, onActionClick, actionConfigError, + disabledReason, accessibilityContext, className, buttonClassName, @@ -86,8 +88,9 @@ export function ActionBar({ const [variableValues, setVariableValues] = useState>({}); const normalizedActionConfigError = actionConfigError?.trim() ?? ""; + const normalizedDisabledReason = disabledReason?.trim() ?? ""; const invalidActions = actions.filter((action) => Boolean(action.validationError?.trim())); - const disableAllActions = isRunning || Boolean(normalizedActionConfigError); + const disableAllActions = isRunning || Boolean(normalizedActionConfigError) || Boolean(normalizedDisabledReason); const closeDialog = useCallback(() => { setDialogAction(null); @@ -163,6 +166,12 @@ export function ActionBar({ ) : null} + {normalizedDisabledReason ? ( + + {normalizedDisabledReason} + + ) : null} + {showValidationErrors && invalidActions.length > 0 ? ( diff --git a/cmd/sgai/webapp/src/components/MarkdownEditor.tsx b/cmd/sgai/webapp/src/components/MarkdownEditor.tsx index 2ae8beb..0030380 100644 --- a/cmd/sgai/webapp/src/components/MarkdownEditor.tsx +++ b/cmd/sgai/webapp/src/components/MarkdownEditor.tsx @@ -39,6 +39,7 @@ interface MarkdownEditorProps { defaultHeight?: number; disabled?: boolean; placeholder?: string; + ariaLabel?: string; workspaceName?: string; fillHeight?: boolean; } @@ -245,6 +246,7 @@ export function MarkdownEditor({ defaultHeight, disabled = false, placeholder, + ariaLabel, workspaceName, fillHeight = false, }: MarkdownEditorProps) { @@ -618,12 +620,14 @@ export function MarkdownEditor({ hideCursorInOverviewRuler: true, readOnly: disabled, domReadOnly: disabled, - padding: { top: 8, bottom: 8 }, - quickSuggestions: workspaceName ? { other: true, strings: true } : false, - wordBasedSuggestions: "off" as const, - suggestOnTriggerCharacters: !!workspaceName, - acceptSuggestionOnEnter: workspaceName ? "on" : "off", - }} + padding: { top: 8, bottom: 8 }, + ariaLabel, + autoIndent: workspaceName ? "full" : "none", + quickSuggestions: workspaceName ? { other: true, strings: true } : false, + wordBasedSuggestions: "off" as const, + suggestOnTriggerCharacters: !!workspaceName, + acceptSuggestionOnEnter: workspaceName ? "on" : "off", + }} /> ) : ( diff --git a/cmd/sgai/webapp/src/components/__tests__/MarkdownEditorShortcut.test.tsx b/cmd/sgai/webapp/src/components/__tests__/MarkdownEditorShortcut.test.tsx index 0cad465..ea4bb8a 100644 --- a/cmd/sgai/webapp/src/components/__tests__/MarkdownEditorShortcut.test.tsx +++ b/cmd/sgai/webapp/src/components/__tests__/MarkdownEditorShortcut.test.tsx @@ -79,6 +79,13 @@ const mockModelsList = mock(() => ({ models: [] })); type RestorableGlobalKey = "fetch" | "EventSource"; let editorValue = ""; +let latestEditorOptions: { + ariaLabel?: string; + autoIndent?: string; + quickSuggestions?: { other: boolean; strings: boolean } | false; + suggestOnTriggerCharacters?: boolean; + acceptSuggestionOnEnter?: "on" | "off"; +} | null = null; let originalFetchDescriptor: PropertyDescriptor | undefined; let originalEventSourceDescriptor: PropertyDescriptor | undefined; @@ -107,7 +114,16 @@ function MockEditor({ value, onChange, onMount }: { value?: string; onChange?: (nextValue: string | undefined) => void; onMount?: (editor: unknown, monaco: unknown) => void; + options?: { + ariaLabel?: string; + autoIndent?: string; + quickSuggestions?: { other: boolean; strings: boolean } | false; + suggestOnTriggerCharacters?: boolean; + acceptSuggestionOnEnter?: "on" | "off"; + }; }) { + latestEditorOptions = arguments[0]?.options ?? null; + useEffect(() => { editorValue = value ?? ""; }, [value]); @@ -262,6 +278,7 @@ function renderInlineForkEditor() { describe("MarkdownEditor submit shortcut integration", () => { beforeEach(() => { editorValue = ""; + latestEditorOptions = null; originalFetchDescriptor = captureGlobalDescriptor("fetch"); originalEventSourceDescriptor = captureGlobalDescriptor("EventSource"); globalThis.fetch = mockFetch as unknown as typeof fetch; @@ -326,6 +343,29 @@ describe("MarkdownEditor submit shortcut integration", () => { }); }); + it("passes an editor aria label through to Monaco", () => { + renderWithProviders( + {}} ariaLabel="Task body" />, + ); + + expect(latestEditorOptions?.ariaLabel).toBe("Task body"); + }); + + it("keeps the body-only inline fork editor out of workspace completion mode", async () => { + renderInlineForkEditor(); + + await waitFor(() => { + expect((screen.getByTestId("monaco-editor-input") as HTMLTextAreaElement).value).toBe("# Goal\n\nDescribe your task"); + }); + + expect(mockAgentsList).not.toHaveBeenCalled(); + expect(mockModelsList).not.toHaveBeenCalled(); + expect(latestEditorOptions?.autoIndent).toBe("none"); + expect(latestEditorOptions?.quickSuggestions).toBe(false); + expect(latestEditorOptions?.suggestOnTriggerCharacters).toBe(false); + expect(latestEditorOptions?.acceptSuggestionOnEnter).toBe("off"); + }); + it("saves GOAL.md from EditGoal when Meta+S is pressed inside the editor", async () => { renderEditGoal(); @@ -350,7 +390,7 @@ describe("MarkdownEditor submit shortcut integration", () => { renderInlineForkEditor(); await waitFor(() => { - expect((screen.getByTestId("monaco-editor-input") as HTMLTextAreaElement).value).toContain("# Goal"); + expect((screen.getByTestId("monaco-editor-input") as HTMLTextAreaElement).value).toBe("# Goal\n\nDescribe your task"); }); const editor = screen.getByTestId("monaco-editor-input") as HTMLTextAreaElement; @@ -358,7 +398,7 @@ describe("MarkdownEditor submit shortcut integration", () => { fireEvent.keyDown(editor, { key: "s", ctrlKey: true, bubbles: true }); await waitFor(() => { - expect(mockFork).toHaveBeenCalledWith(expect.stringContaining("# Goal")); + expect(mockFork).toHaveBeenCalledWith("# Goal\n\nDescribe your task"); }); await waitFor(() => { diff --git a/cmd/sgai/webapp/src/hooks/__tests__/useAdhocRun.test.ts b/cmd/sgai/webapp/src/hooks/__tests__/useAdhocRun.test.ts index 3560900..e0838b6 100644 --- a/cmd/sgai/webapp/src/hooks/__tests__/useAdhocRun.test.ts +++ b/cmd/sgai/webapp/src/hooks/__tests__/useAdhocRun.test.ts @@ -299,6 +299,28 @@ describe("useAdhocRun", () => { expect(result.current.promptHistory).toContain("test prompt"); }); + + it("blocks adhoc runs when basename-only mutations are disabled", async () => { + const { result } = renderHook(() => + useAdhocRun({ + workspaceName: "test-ws", + skipModelsFetch: true, + disableMutationsReason: "Duplicate-name route", + }) + ); + + act(() => { + result.current.setPrompt("test prompt"); + result.current.setSelectedModel("model-1"); + }); + + await act(async () => { + await result.current.startRun(); + }); + + expect(mockAdhoc).not.toHaveBeenCalled(); + expect(result.current.runError).toBe("Duplicate-name route"); + }); }); describe("startActionRun", () => { @@ -332,6 +354,23 @@ describe("useAdhocRun", () => { variables: {}, }); }); + + it("blocks named actions when basename-only mutations are disabled", async () => { + const { result } = renderHook(() => + useAdhocRun({ + workspaceName: "test-ws", + skipModelsFetch: true, + disableMutationsReason: "Duplicate-name route", + }) + ); + + await act(async () => { + await result.current.startActionRun("Run Tests", { Branch: "main" }); + }); + + expect(mockActionRun).not.toHaveBeenCalled(); + expect(result.current.runError).toBe("Duplicate-name route"); + }); }); describe("models fetching", () => { @@ -358,6 +397,21 @@ describe("useAdhocRun", () => { }); }); + describe("status polling", () => { + it("skips the initial adhoc status fetch when skipStatusFetch is true", async () => { + renderHook(() => + useAdhocRun({ workspaceName: "test-ws", skipModelsFetch: true, skipStatusFetch: true } as any) + ); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mockAdhocStatus).not.toHaveBeenCalled(); + }); + }); + describe("stopRun", () => { it("does nothing when not running", async () => { const { result } = renderHook(() => @@ -370,6 +424,34 @@ describe("useAdhocRun", () => { expect(mockAdhocStop).not.toHaveBeenCalled(); }); + + it("blocks stop requests when basename-only mutations are disabled", async () => { + mockAdhocStatus.mockImplementationOnce(() => Promise.resolve({ output: "still running", running: true })); + + const { result } = renderHook(() => + useAdhocRun({ + workspaceName: "test-ws", + skipModelsFetch: true, + disableMutationsReason: "Duplicate-name route", + }) + ); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + await waitFor(() => { + expect(result.current.isRunning).toBe(true); + }); + + await act(async () => { + await result.current.stopRun(); + }); + + expect(mockAdhocStop).not.toHaveBeenCalled(); + expect(result.current.runError).toBe("Duplicate-name route"); + }); }); describe("localStorage persistence", () => { diff --git a/cmd/sgai/webapp/src/hooks/useAdhocRun.ts b/cmd/sgai/webapp/src/hooks/useAdhocRun.ts index 453e7d5..b8839ff 100644 --- a/cmd/sgai/webapp/src/hooks/useAdhocRun.ts +++ b/cmd/sgai/webapp/src/hooks/useAdhocRun.ts @@ -32,6 +32,10 @@ export interface UseAdhocRunOptions { currentModel?: string; /** When true, skip fetching model list (AdhocOutput uses a text input) */ skipModelsFetch?: boolean; + /** When true, skip background adhoc status fetches for dir-qualified duplicate routes */ + skipStatusFetch?: boolean; + /** When set, block basename-only adhoc mutations and surface the reason to the UI */ + disableMutationsReason?: string; } export interface UseAdhocRunResult { @@ -60,6 +64,8 @@ export function useAdhocRun({ workspaceName, currentModel, skipModelsFetch = false, + skipStatusFetch = false, + disableMutationsReason, }: UseAdhocRunOptions): UseAdhocRunResult { const [models, setModels] = useState(null); const [modelsLoading, setModelsLoading] = useState(!skipModelsFetch); @@ -78,6 +84,7 @@ export function useAdhocRun({ const [promptHistory, setPromptHistory] = useState(() => readLocalStorage(storageKey(workspaceName, "history"), []), ); + const normalizedDisableMutationsReason = disableMutationsReason?.trim() ?? ""; const outputRef = useRef(null); const pollTimerRef = useRef | null>(null); @@ -219,6 +226,7 @@ export function useAdhocRun({ }, [stopPolling]); useEffect(() => { + if (skipStatusFetch) return; if (!workspaceName || runWorkspaceName !== workspaceName) return; let cancelled = false; api.workspaces.adhocStatus(workspaceName).then((status) => { @@ -231,7 +239,7 @@ export function useAdhocRun({ } }).catch(() => {}); return () => { cancelled = true; }; - }, [workspaceName, runWorkspaceName, startStatusPolling]); + }, [workspaceName, runWorkspaceName, skipStatusFetch, startStatusPolling]); const executeRun = useCallback( async ({ @@ -248,6 +256,13 @@ export function useAdhocRun({ const trimmedWorkspaceName = targetWorkspaceName.trim(); if (!trimmedWorkspaceName || isRunning) return; + if (normalizedDisableMutationsReason) { + stopPolling(); + setIsRunning(false); + setRunError(normalizedDisableMutationsReason); + return; + } + stopPolling(); setRunWorkspaceName(trimmedWorkspaceName); setIsRunning(true); @@ -274,7 +289,7 @@ export function useAdhocRun({ setIsRunning(false); } }, - [addToHistory, isRunning, startStatusPolling, stopPolling], + [addToHistory, isRunning, normalizedDisableMutationsReason, startStatusPolling, stopPolling], ); const startRun = useCallback( @@ -315,6 +330,13 @@ export function useAdhocRun({ const stopRun = useCallback(async () => { if (!runWorkspaceName || !isRunning) return; + if (normalizedDisableMutationsReason) { + stopPolling(); + setIsRunning(false); + setRunError(normalizedDisableMutationsReason); + return; + } + try { await api.workspaces.adhocStop(runWorkspaceName); stopPolling(); @@ -327,7 +349,7 @@ export function useAdhocRun({ setRunError("Failed to stop ad-hoc prompt"); } } - }, [isRunning, runWorkspaceName, stopPolling]); + }, [isRunning, normalizedDisableMutationsReason, runWorkspaceName, stopPolling]); const handleSubmit = useCallback( (event: React.FormEvent) => { diff --git a/cmd/sgai/webapp/src/hooks/useResponseForm.ts b/cmd/sgai/webapp/src/hooks/useResponseForm.ts index a233257..8b0a6e4 100644 --- a/cmd/sgai/webapp/src/hooks/useResponseForm.ts +++ b/cmd/sgai/webapp/src/hooks/useResponseForm.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { api, ApiError } from "@/lib/api"; +import { duplicateRouteResponseSubmissionDisabledReason } from "@/lib/duplicate-route-mutations"; import { useWorkspacePageState } from "@/lib/workspace-page-state"; import type { ApiPendingQuestionResponse, ApiWorkspaceEntry } from "@/types"; @@ -9,13 +10,13 @@ interface StoredResponseState { promptToken: string; } -function getStorageKey(prefix: string, workspaceName: string): string { - return `${prefix}${workspaceName}`; +function getStorageKey(prefix: string, workspaceStorageKey: string): string { + return `${prefix}${workspaceStorageKey}`; } -function loadStoredState(prefix: string, workspaceName: string): StoredResponseState | null { +function loadStoredState(prefix: string, workspaceStorageKey: string): StoredResponseState | null { try { - const stored = sessionStorage.getItem(getStorageKey(prefix, workspaceName)); + const stored = sessionStorage.getItem(getStorageKey(prefix, workspaceStorageKey)); if (stored) { return JSON.parse(stored) as StoredResponseState; } @@ -25,17 +26,17 @@ function loadStoredState(prefix: string, workspaceName: string): StoredResponseS return null; } -function saveStoredState(prefix: string, workspaceName: string, state: StoredResponseState): void { +function saveStoredState(prefix: string, workspaceStorageKey: string, state: StoredResponseState): void { try { - sessionStorage.setItem(getStorageKey(prefix, workspaceName), JSON.stringify(state)); + sessionStorage.setItem(getStorageKey(prefix, workspaceStorageKey), JSON.stringify(state)); } catch { // Ignore storage errors } } -function clearStoredState(prefix: string, workspaceName: string): void { +function clearStoredState(prefix: string, workspaceStorageKey: string): void { try { - sessionStorage.removeItem(getStorageKey(prefix, workspaceName)); + sessionStorage.removeItem(getStorageKey(prefix, workspaceStorageKey)); } catch { // Ignore } @@ -43,6 +44,7 @@ function clearStoredState(prefix: string, workspaceName: string): void { interface UseResponseFormOptions { workspaceName: string; + workspaceDir?: string; storagePrefix: string; active: boolean; onQuestionMissing?: () => void; @@ -56,6 +58,7 @@ interface UseResponseFormReturn { error: Error | null; submitting: boolean; submitError: string | null; + submitDisabledReason: string | null; selections: Record; otherText: string; setOtherText: (text: string) => void; @@ -65,6 +68,7 @@ interface UseResponseFormReturn { export function useResponseForm({ workspaceName, + workspaceDir, storagePrefix, active, onQuestionMissing, @@ -77,7 +81,16 @@ export function useResponseForm({ const hasUnsavedChangesRef = useRef(false); const previousPromptTokenRef = useRef(null); - const { workspace, fetchStatus } = useWorkspacePageState(workspaceName); + const normalizedWorkspaceDir = workspaceDir?.trim() ?? ""; + const submitDisabledReason = normalizedWorkspaceDir + ? duplicateRouteResponseSubmissionDisabledReason + : null; + const workspaceStorageKey = normalizedWorkspaceDir ? `${workspaceName}|${normalizedWorkspaceDir}` : workspaceName; + const { workspace, fetchStatus } = useWorkspacePageState( + normalizedWorkspaceDir + ? { name: workspaceName, dir: normalizedWorkspaceDir } + : workspaceName, + ); const question = workspace?.pendingQuestion ?? null; const promptToken = question?.promptToken ?? null; const loading = fetchStatus === "fetching" && workspace === null; @@ -97,7 +110,7 @@ export function useResponseForm({ if (previousPromptTokenRef.current !== promptToken) { previousPromptTokenRef.current = promptToken; - const stored = loadStoredState(storagePrefix, workspaceName); + const stored = loadStoredState(storagePrefix, workspaceStorageKey); if (stored && stored.promptToken === promptToken) { setSelections(stored.selections); setOtherText(stored.otherText); @@ -106,7 +119,7 @@ export function useResponseForm({ setOtherText(""); } } - }, [active, workspaceName, storagePrefix, promptToken, workspace, onQuestionMissing]); + }, [active, workspaceStorageKey, storagePrefix, promptToken, workspace, onQuestionMissing]); useEffect(() => { if (promptToken === null) return; @@ -115,12 +128,12 @@ export function useResponseForm({ const hasText = otherText.trim().length > 0; hasUnsavedChangesRef.current = hasSelections || hasText; - saveStoredState(storagePrefix, workspaceName, { + saveStoredState(storagePrefix, workspaceStorageKey, { selections, otherText, promptToken, }); - }, [selections, otherText, promptToken, workspaceName, storagePrefix]); + }, [selections, otherText, promptToken, workspaceStorageKey, storagePrefix]); useEffect(() => { function handleBeforeUnload(e: BeforeUnloadEvent) { @@ -160,6 +173,11 @@ export function useResponseForm({ if (!question || submitting) return; + if (submitDisabledReason) { + setSubmitError(submitDisabledReason); + return; + } + setSubmitting(true); setSubmitError(null); @@ -175,7 +193,7 @@ export function useResponseForm({ selectedChoices: allSelectedChoices, }); - clearStoredState(storagePrefix, workspaceName); + clearStoredState(storagePrefix, workspaceStorageKey); hasUnsavedChangesRef.current = false; onSubmitSuccess?.(); } catch (err: unknown) { @@ -190,7 +208,7 @@ export function useResponseForm({ setSubmitting(false); } }, - [question, submitting, selections, otherText, workspaceName, storagePrefix, onSubmitSuccess], + [question, submitting, submitDisabledReason, selections, otherText, workspaceName, workspaceStorageKey, storagePrefix, onSubmitSuccess], ); return { @@ -200,6 +218,7 @@ export function useResponseForm({ error, submitting, submitError, + submitDisabledReason, selections, otherText, setOtherText, diff --git a/cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts b/cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts index f2302b0..cbe57b5 100644 --- a/cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts +++ b/cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts @@ -69,13 +69,18 @@ async function advanceTimersAndFlush(timeMs: number): Promise { const mockFetch = mock(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" }))); -function WorkspacePageProbe({ workspaceName }: { workspaceName: string }) { - const { workspace, fetchStatus } = useWorkspacePageState(workspaceName); +function WorkspacePageProbe({ + workspaceTarget, +}: { + workspaceTarget: string | { name: string; dir?: string }; +}) { + const { workspace, fetchStatus } = useWorkspacePageState(workspaceTarget as never); return createElement( "div", null, createElement("span", { "data-testid": "workspace-name" }, workspace?.name ?? ""), + createElement("span", { "data-testid": "workspace-dir" }, workspace?.dir ?? ""), createElement("span", { "data-testid": "fetch-status" }, fetchStatus), ); } @@ -99,7 +104,7 @@ describe("workspace-page-state store", () => { it("queues a second refresh request while the current fetch is still running", async () => { const firstResponse = deferredValue(); - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await waitFor(() => { expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); @@ -135,7 +140,7 @@ describe("workspace-page-state store", () => { .mockImplementationOnce(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" }))) .mockImplementationOnce(() => secondResponse.promise); - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await waitFor(() => { expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); @@ -160,7 +165,7 @@ describe("workspace-page-state store", () => { it("stops polling and ignores refresh requests after the last subscriber unsubscribes", async () => { vi.useFakeTimers(); - const { unmount } = render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + const { unmount } = render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await waitFor(() => { expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); @@ -179,7 +184,7 @@ describe("workspace-page-state store", () => { }); it("ignores generic reload SSE events after the workspace page has loaded", async () => { - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await waitFor(() => { expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); @@ -199,7 +204,7 @@ describe("workspace-page-state store", () => { }); it("refreshes only for matching workspace-dir SSE events and keeps state fetches headerless", async () => { - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await waitFor(() => { expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); @@ -223,7 +228,7 @@ describe("workspace-page-state store", () => { it("stops visible polling once SSE is connected and resumes fallback polling after disconnect", async () => { vi.useFakeTimers(); - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await advanceTimersAndFlush(0); expect(mockFetch.mock.calls.length).toBe(1); @@ -245,7 +250,7 @@ describe("workspace-page-state store", () => { it("does not fall back to visible polling while an idle EventSource still exists", async () => { vi.useFakeTimers(); - render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + render(createElement(WorkspacePageProbe, { workspaceTarget: "test-workspace" })); await advanceTimersAndFlush(0); expect(mockFetch.mock.calls.length).toBe(1); @@ -256,4 +261,31 @@ describe("workspace-page-state store", () => { expect(mockFetch.mock.calls.length).toBe(1); expect(MockEventSource.instances.length).toBe(1); }); + + it("loads a dir-qualified workspace target from the full factory state endpoint", async () => { + mockFetch.mockImplementation((input) => { + if (input === "/api/v1/state") { + return Promise.resolve(createResponse({ + workspaces: [ + { name: "shared-ws", dir: "/tmp/first/shared-ws" }, + { name: "shared-ws", dir: "/tmp/second/shared-ws" }, + ], + })); + } + + throw new Error(`Unexpected fetch target: ${String(input)}`); + }); + + render(createElement(WorkspacePageProbe, { + workspaceTarget: { name: "shared-ws", dir: "/tmp/second/shared-ws" }, + })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("shared-ws"); + expect(screen.getByTestId("workspace-dir").textContent).toBe("/tmp/second/shared-ws"); + expect(screen.getByTestId("fetch-status").textContent).toBe("idle"); + }); + + expect(mockFetch.mock.calls[0]?.[0]).toBe("/api/v1/state"); + }); }); diff --git a/cmd/sgai/webapp/src/lib/active-agents.ts b/cmd/sgai/webapp/src/lib/active-agents.ts new file mode 100644 index 0000000..bb1534d --- /dev/null +++ b/cmd/sgai/webapp/src/lib/active-agents.ts @@ -0,0 +1,23 @@ +import type { ApiWorkspaceEntry } from "@/types"; + +type ActiveAgentSource = Pick; + +export function normalizeActiveAgents({ currentAgent, activeAgents }: ActiveAgentSource): string[] { + const normalizedActiveAgents = Array.from(new Set((activeAgents ?? []) + .map((agent) => agent.trim()) + .filter(Boolean))); + + if (normalizedActiveAgents.length > 0) { + return normalizedActiveAgents; + } + + const normalizedCurrentAgent = currentAgent.trim(); + if (!normalizedCurrentAgent || normalizedCurrentAgent === "Unknown") { + return []; + } + + return Array.from(new Set(normalizedCurrentAgent + .split(",") + .map((agent) => agent.trim()) + .filter(Boolean))); +} diff --git a/cmd/sgai/webapp/src/lib/duplicate-route-mutations.ts b/cmd/sgai/webapp/src/lib/duplicate-route-mutations.ts new file mode 100644 index 0000000..2b19dfe --- /dev/null +++ b/cmd/sgai/webapp/src/lib/duplicate-route-mutations.ts @@ -0,0 +1,5 @@ +export const duplicateRouteWorkspaceControlsDisabledReason = "Some interactive controls are unavailable on duplicate-name workspace routes because the current API still targets workspaces by basename only."; + +export const duplicateRouteActionButtonsDisabledReason = "Action buttons are unavailable on duplicate-name workspace routes because the current API still targets workspaces by basename only."; + +export const duplicateRouteResponseSubmissionDisabledReason = "Response submission is unavailable on duplicate-name workspace routes because the current API still targets workspaces by basename only."; diff --git a/cmd/sgai/webapp/src/lib/workspace-identity.ts b/cmd/sgai/webapp/src/lib/workspace-identity.ts index 32e44ad..18cc00b 100644 --- a/cmd/sgai/webapp/src/lib/workspace-identity.ts +++ b/cmd/sgai/webapp/src/lib/workspace-identity.ts @@ -5,6 +5,12 @@ type WorkspaceIdentity = Pick; type WorkspaceLabelSource = WorkspaceIdentity & Pick; +interface WorkspacePathOptions { + workspaceDir?: string; +} + +const workspaceDirQueryParam = "workspaceDir"; + export function getWorkspaceBaseLabel(workspace: WorkspaceLabelSource): string { const computedTitle = workspace.computedTitle?.trim(); @@ -56,13 +62,14 @@ function buildGroupDisambiguators(workspaces: WorkspaceIdentity[]): Map { +export function buildWorkspaceNameDisambiguators(workspaces: WorkspaceLabelSource[]): Map { const grouped = new Map(); for (const workspace of workspaces) { - const existing = grouped.get(workspace.name) ?? []; + const baseLabel = getWorkspaceBaseLabel(workspace); + const existing = grouped.get(baseLabel) ?? []; existing.push(workspace); - grouped.set(workspace.name, existing); + grouped.set(baseLabel, existing); } const disambiguators = new Map(); @@ -81,6 +88,30 @@ export function buildWorkspaceNameDisambiguators(workspaces: WorkspaceIdentity[] return disambiguators; } +export function buildWorkspaceRouteDisambiguators(workspaces: WorkspaceIdentity[]): Set { + const grouped = new Map(); + + for (const workspace of workspaces) { + const existing = grouped.get(workspace.name) ?? []; + existing.push(workspace); + grouped.set(workspace.name, existing); + } + + const disambiguators = new Set(); + + for (const [, group] of grouped) { + if (group.length < 2) { + continue; + } + + for (const workspace of group) { + disambiguators.add(workspace.dir); + } + } + + return disambiguators; +} + export function getWorkspaceDisplayLabel( workspace: WorkspaceLabelSource, workspaceNameDisambiguators: Map, @@ -95,22 +126,61 @@ export function getWorkspaceDisplayLabel( return `${baseLabel} · ${disambiguator}`; } -export function buildWorkspacePath(workspace: WorkspaceIdentity, suffix = ""): string { +export function buildWorkspacePathFromName( + workspaceName: string, + suffix = "", + options: WorkspacePathOptions = {}, +): string { const normalizedSuffix = suffix.replace(/^\/+/, ""); - const basePath = `/workspaces/${encodeURIComponent(workspace.name)}`; - return normalizedSuffix ? `${basePath}/${normalizedSuffix}` : basePath; + const basePath = `/workspaces/${encodeURIComponent(workspaceName)}`; + const path = normalizedSuffix ? `${basePath}/${normalizedSuffix}` : basePath; + const workspaceDir = options.workspaceDir?.trim(); + + if (!workspaceDir) { + return path; + } + + const searchParams = new URLSearchParams({ [workspaceDirQueryParam]: workspaceDir }); + return `${path}?${searchParams.toString()}`; +} + +export function buildWorkspacePath( + workspace: WorkspaceIdentity, + suffix = "", + options: WorkspacePathOptions = {}, +): string { + return buildWorkspacePathFromName(workspace.name, suffix, options); } export function buildWorkspaceGoalEditPath(workspace: WorkspaceIdentity): string { - return `/workspaces/${encodeURIComponent(workspace.name)}/goal/edit`; + return buildWorkspacePathFromName(workspace.name, "goal/edit"); +} + +export function buildWorkspacePathWithDisambiguator( + workspace: WorkspaceIdentity, + workspaceRouteDisambiguators: Set, + suffix = "", +): string { + return buildWorkspacePath(workspace, suffix, { + workspaceDir: workspaceRouteDisambiguators.has(workspace.dir) ? workspace.dir : undefined, + }); +} + +export function readWorkspaceDirFromSearchParams(searchParams: URLSearchParams): string { + return searchParams.get(workspaceDirQueryParam)?.trim() ?? ""; } -export function resolveWorkspaceByName( +export function resolveWorkspaceTarget( workspaces: T[], workspaceName: string, + workspaceDir = "", ): T | null { const matches = workspaces.filter((workspace) => workspace.name === workspaceName); + if (workspaceDir) { + return matches.find((workspace) => workspace.dir === workspaceDir) ?? null; + } + return matches.length === 1 ? (matches[0] ?? null) : null; } diff --git a/cmd/sgai/webapp/src/lib/workspace-page-state.ts b/cmd/sgai/webapp/src/lib/workspace-page-state.ts index e7747eb..d1873e7 100644 --- a/cmd/sgai/webapp/src/lib/workspace-page-state.ts +++ b/cmd/sgai/webapp/src/lib/workspace-page-state.ts @@ -2,6 +2,15 @@ import { useSyncExternalStore } from "react"; import type { ApiWorkspaceEntry } from "@/types"; import type { FetchStatus } from "./factory-state"; +export interface WorkspacePageTarget { + name: string; + dir?: string; +} + +interface FactoryStateResponse { + workspaces: ApiWorkspaceEntry[] | null; +} + interface WorkspacePageSnapshot { workspace: ApiWorkspaceEntry | null; fetchStatus: FetchStatus; @@ -20,7 +29,24 @@ const SSE_BASE_BACKOFF_MS = 1000; const SSE_MAX_BACKOFF_MS = 30000; const DEBOUNCE_MS = 300; -function createWorkspacePageStore(workspaceName: string, removeStore: () => void) { +function normalizeWorkspaceTarget(workspaceTarget: string | WorkspacePageTarget): WorkspacePageTarget { + if (typeof workspaceTarget === "string") { + return { name: workspaceTarget.trim() }; + } + + return { + name: workspaceTarget.name.trim(), + dir: workspaceTarget.dir?.trim() ?? "", + }; +} + +function workspaceTargetKey(workspaceTarget: WorkspacePageTarget): string { + return workspaceTarget.dir ? `${workspaceTarget.name}|${workspaceTarget.dir}` : workspaceTarget.name; +} + +function createWorkspacePageStore(workspaceTarget: WorkspacePageTarget, removeStore: () => void) { + const workspaceName = workspaceTarget.name; + const workspaceDir = workspaceTarget.dir?.trim() ?? ""; let snapshot: WorkspacePageSnapshot = { workspace: null, fetchStatus: "idle", @@ -85,7 +111,9 @@ function createWorkspacePageStore(workspaceName: string, removeStore: () => void try { const response = await fetch( - `/api/v1/workspaces/${encodeURIComponent(workspaceName)}/state`, + workspaceDir + ? "/api/v1/state" + : `/api/v1/workspaces/${encodeURIComponent(workspaceName)}/state`, ); if (isDestroyed) return; if (!response.ok) { @@ -93,8 +121,17 @@ function createWorkspacePageStore(workspaceName: string, removeStore: () => void return; } - const data = (await response.json()) as ApiWorkspaceEntry; + const data = workspaceDir + ? ((await response.json()) as FactoryStateResponse).workspaces?.find( + (workspace) => workspace.name === workspaceName && workspace.dir === workspaceDir, + ) ?? null + : (await response.json()) as ApiWorkspaceEntry; if (isDestroyed) return; + if (data === null) { + updateSnapshot({ fetchStatus: snapshot.workspace === null ? "error" : "idle" }); + return; + } + updateSnapshot({ workspace: data, fetchStatus: "idle", @@ -160,7 +197,7 @@ function createWorkspacePageStore(workspaceName: string, removeStore: () => void if (!payload.workspace) { return false; } - return payload.workspace === workspaceName || payload.workspace === snapshot.workspace?.dir; + return payload.workspace === workspaceName || payload.workspace === workspaceDir || payload.workspace === snapshot.workspace?.dir; } function connectSSESignal() { @@ -306,24 +343,21 @@ type WorkspacePageStore = ReturnType; const storeInstances = new Map(); -function normalizeWorkspaceName(workspaceName: string): string { - return workspaceName.trim(); -} - -function getWorkspacePageStore(workspaceName: string): WorkspacePageStore { - const normalizedName = normalizeWorkspaceName(workspaceName); - const existingStore = storeInstances.get(normalizedName); +function getWorkspacePageStore(workspaceTarget: string | WorkspacePageTarget): WorkspacePageStore { + const normalizedTarget = normalizeWorkspaceTarget(workspaceTarget); + const targetKey = workspaceTargetKey(normalizedTarget); + const existingStore = storeInstances.get(targetKey); if (existingStore) { return existingStore; } let createdStore!: WorkspacePageStore; - createdStore = createWorkspacePageStore(normalizedName, () => { - if (storeInstances.get(normalizedName) === createdStore) { - storeInstances.delete(normalizedName); + createdStore = createWorkspacePageStore(normalizedTarget, () => { + if (storeInstances.get(targetKey) === createdStore) { + storeInstances.delete(targetKey); } }); - storeInstances.set(normalizedName, createdStore); + storeInstances.set(targetKey, createdStore); return createdStore; } @@ -334,17 +368,17 @@ export function resetWorkspacePageStateStores(): void { storeInstances.clear(); } -export function triggerWorkspacePageRefresh(workspaceName: string): void { - const normalizedName = normalizeWorkspaceName(workspaceName); - const store = storeInstances.get(normalizedName); +export function triggerWorkspacePageRefresh(workspaceTarget: string | WorkspacePageTarget): void { + const normalizedTarget = normalizeWorkspaceTarget(workspaceTarget); + const store = storeInstances.get(workspaceTargetKey(normalizedTarget)); if (!store) { return; } store.triggerRefresh(); } -export function useWorkspacePageState(workspaceName: string): WorkspacePageSnapshot { - const store = getWorkspacePageStore(workspaceName); +export function useWorkspacePageState(workspaceTarget: string | WorkspacePageTarget): WorkspacePageSnapshot { + const store = getWorkspacePageStore(workspaceTarget); return useSyncExternalStore( store.subscribe, store.getSnapshot, diff --git a/cmd/sgai/webapp/src/pages/Dashboard.tsx b/cmd/sgai/webapp/src/pages/Dashboard.tsx index 1f5d8e5..a7c12da 100644 --- a/cmd/sgai/webapp/src/pages/Dashboard.tsx +++ b/cmd/sgai/webapp/src/pages/Dashboard.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback, useMemo, type ReactNode, type CSSProperties } from "react"; -import { useParams, useNavigate, Link } from "react-router"; +import { useParams, useNavigate, Link, useSearchParams } from "react-router"; import { ScrollArea } from "@/components/ui/scroll-area"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -30,10 +30,12 @@ import type { ApiWorkspaceEntry } from "@/lib/factory-state"; import { getWorkspaceBaseLabel, buildWorkspaceNameDisambiguators, - buildWorkspacePath, + buildWorkspaceRouteDisambiguators, + buildWorkspacePathWithDisambiguator, getWorkspaceDisplayLabel, isSameWorkspace, - resolveWorkspaceByName, + readWorkspaceDirFromSearchParams, + resolveWorkspaceTarget, } from "@/lib/workspace-identity"; import { sortByVisibleLabel } from "@/lib/workspace-sort"; import type { ApiRepositoryOperation } from "@/types"; @@ -313,7 +315,7 @@ function WorkspaceTreeActionSlot({ triggerLabelSuffix, onCompleted, }: WorkspaceTreeActionSlotProps) { - if (!usesLeftTreeDetachSlot(workspace)) { + if (!usesLeftTreeDetachSlot(workspace) || triggerLabelSuffix) { return null; } @@ -349,7 +351,7 @@ function WorkspaceTreeTrailingAction({ triggerLabelSuffix, onCompleted, }: WorkspaceTreeTrailingActionProps) { - const showTrailingAction = Boolean(workspace && usesTrailingTreeAction(workspace)); + const showTrailingAction = Boolean(workspace && usesTrailingTreeAction(workspace) && !triggerLabelSuffix); return ( @@ -376,6 +378,7 @@ interface ForkItemProps { workspaceLookup: Map; rootWorkspace?: Pick; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function ForkItem({ @@ -384,6 +387,7 @@ function ForkItem({ workspaceLookup, rootWorkspace, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: ForkItemProps) { const navigate = useNavigate(); const forkSelected = isSameWorkspace(fork, selectedWorkspace); @@ -393,9 +397,9 @@ function ForkItem({ const { rowTooltipOpen, handleRowTooltipOpenChange, handleLinkFocus, handleLinkBlur } = useTreeRowTooltipState(); const handleActionCompleted = useCallback(() => { if (isSameWorkspace(fork, selectedWorkspace) && rootWorkspace) { - navigate(buildWorkspacePath(rootWorkspace, "forks")); + navigate(buildWorkspacePathWithDisambiguator(rootWorkspace, workspaceRouteDisambiguators, "forks")); } - }, [fork, navigate, rootWorkspace, selectedWorkspace]); + }, [fork, navigate, rootWorkspace, selectedWorkspace, workspaceRouteDisambiguators]); return ( @@ -417,7 +421,7 @@ function ForkItem({ )} > @@ -453,6 +457,7 @@ interface WorkspaceTreeItemProps { selectedWorkspace: ApiWorkspaceEntry | null; workspaceLookup: Map; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function WorkspaceTreeItem({ @@ -460,6 +465,7 @@ function WorkspaceTreeItem({ selectedWorkspace, workspaceLookup, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: WorkspaceTreeItemProps) { const navigate = useNavigate(); const fullWorkspace = workspaceLookup.get(workspace.dir); @@ -525,7 +531,7 @@ function WorkspaceTreeItem({ )} > @@ -564,6 +570,7 @@ function WorkspaceTreeItem({ workspaceLookup={workspaceLookup} rootWorkspace={fullWorkspace ?? workspace} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> ))} @@ -578,6 +585,7 @@ interface InProgressItemProps { selectedWorkspace: ApiWorkspaceEntry | null; workspaceLookup: Map; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function InProgressItem({ @@ -585,6 +593,7 @@ function InProgressItem({ selectedWorkspace, workspaceLookup, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: InProgressItemProps) { const isSelected = isSameWorkspace(workspace, selectedWorkspace); const fullWorkspace = workspaceLookup.get(workspace.dir); @@ -606,7 +615,7 @@ function InProgressItem({ )} > @@ -639,6 +648,7 @@ interface PinnedTreeItemProps { workspaceLookup: Map; pinnedForks: ForkEntry[]; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function PinnedTreeItem({ @@ -647,6 +657,7 @@ function PinnedTreeItem({ workspaceLookup, pinnedForks, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: PinnedTreeItemProps) { const navigate = useNavigate(); const fullWorkspace = workspaceLookup.get(workspace.dir); @@ -710,7 +721,7 @@ function PinnedTreeItem({ )} > @@ -749,6 +760,7 @@ function PinnedTreeItem({ workspaceLookup={workspaceLookup} rootWorkspace={fullWorkspace ?? workspace} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> ))} @@ -764,6 +776,7 @@ interface OrphanPinnedForkItemProps { selectedWorkspace: ApiWorkspaceEntry | null; workspaceLookup: Map; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function OrphanPinnedForkItem({ @@ -772,6 +785,7 @@ function OrphanPinnedForkItem({ selectedWorkspace, workspaceLookup, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: OrphanPinnedForkItemProps) { const navigate = useNavigate(); const forkSelected = isSameWorkspace(fork, selectedWorkspace); @@ -788,9 +802,9 @@ function OrphanPinnedForkItem({ const { rowTooltipOpen, handleRowTooltipOpenChange, handleLinkFocus, handleLinkBlur } = useTreeRowTooltipState(); const handleActionCompleted = useCallback(() => { if (isSameWorkspace(fork, selectedWorkspace)) { - navigate(buildWorkspacePath(rootWorkspace, "forks")); + navigate(buildWorkspacePathWithDisambiguator(rootWorkspace, workspaceRouteDisambiguators, "forks")); } - }, [fork, navigate, rootWorkspace, selectedWorkspace]); + }, [fork, navigate, rootWorkspace, selectedWorkspace, workspaceRouteDisambiguators]); return ( @@ -813,7 +827,7 @@ function OrphanPinnedForkItem({ )} > @@ -851,6 +865,7 @@ interface PinnedSectionProps { workspaceLookup: Map; forkParentLookup: Map; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function PinnedSection({ @@ -859,6 +874,7 @@ function PinnedSection({ workspaceLookup, forkParentLookup, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: PinnedSectionProps) { const pinned = useMemo(() => { return workspaces.filter((w) => w.pinned); @@ -924,6 +940,7 @@ function PinnedSection({ workspaceLookup={workspaceLookup} pinnedForks={pinnedRootsAndForks.forkGroups.get(item.workspace.dir) || []} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> ) : ( ))} @@ -945,6 +963,7 @@ interface InProgressSectionProps { selectedWorkspace: ApiWorkspaceEntry | null; workspaceLookup: Map; workspaceNameDisambiguators: Map; + workspaceRouteDisambiguators: Set; } function InProgressSection({ @@ -952,6 +971,7 @@ function InProgressSection({ selectedWorkspace, workspaceLookup, workspaceNameDisambiguators, + workspaceRouteDisambiguators, }: InProgressSectionProps) { const inProgress = useMemo(() => { return workspaces.filter((w) => (w.inProgress || w.running) && !w.pinned); @@ -977,6 +997,7 @@ function InProgressSection({ selectedWorkspace={selectedWorkspace} workspaceLookup={workspaceLookup} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> ))} @@ -993,6 +1014,9 @@ function WorkspaceList({ workspaces, selectedWorkspace }: WorkspaceListProps) { const workspaceNameDisambiguators = useMemo(() => { return buildWorkspaceNameDisambiguators(workspaces); }, [workspaces]); + const workspaceRouteDisambiguators = useMemo(() => { + return buildWorkspaceRouteDisambiguators(workspaces); + }, [workspaces]); const workspaceLookup = useMemo(() => { const map = new Map(); @@ -1034,12 +1058,14 @@ function WorkspaceList({ workspaces, selectedWorkspace }: WorkspaceListProps) { workspaceLookup={workspaceLookup} forkParentLookup={forkParentLookup} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> {deduplicatedWorkspaces.length > 0 ? ( @@ -1050,6 +1076,7 @@ function WorkspaceList({ workspaces, selectedWorkspace }: WorkspaceListProps) { selectedWorkspace={selectedWorkspace} workspaceLookup={workspaceLookup} workspaceNameDisambiguators={workspaceNameDisambiguators} + workspaceRouteDisambiguators={workspaceRouteDisambiguators} /> )) ) : ( @@ -1067,6 +1094,10 @@ interface SidebarHeaderIndicatorsProps { function SidebarHeaderIndicators({ workspaces }: SidebarHeaderIndicatorsProps) { const navigate = useNavigate(); const allWorkspaces = useMemo(() => collectWorkspaceStatusEntries(workspaces), [workspaces]); + const workspaceRouteDisambiguators = useMemo( + () => buildWorkspaceRouteDisambiguators(allWorkspaces), + [allWorkspaces], + ); const needsInputCount = useMemo( () => allWorkspaces.filter((w) => w.needsInput).length, @@ -1081,9 +1112,9 @@ function SidebarHeaderIndicators({ workspaces }: SidebarHeaderIndicatorsProps) { const handleInboxClick = useCallback(() => { const firstNeedsInput = allWorkspaces.find((w) => w.needsInput); if (firstNeedsInput) { - navigate(buildWorkspacePath(firstNeedsInput, "respond")); + navigate(buildWorkspacePathWithDisambiguator(firstNeedsInput, workspaceRouteDisambiguators, "respond")); } - }, [allWorkspaces, navigate]); + }, [allWorkspaces, navigate, workspaceRouteDisambiguators]); return (
@@ -1151,17 +1182,19 @@ interface DashboardContentProps { function DashboardContent({ children, onSidebarResizeMouseDown }: DashboardContentProps): JSX.Element { const { name: selectedName } = useParams<{ name: string }>(); + const [searchParams] = useSearchParams(); const navigate = useNavigate(); const { setOpenMobile } = useSidebar(); const { workspaces, fetchStatus } = useFactoryState(); + const selectedWorkspaceDir = readWorkspaceDirFromSearchParams(searchParams); const selectedWorkspace = useMemo(() => { if (!selectedName) { return null; } - return resolveWorkspaceByName(workspaces, selectedName); - }, [selectedName, workspaces]); + return resolveWorkspaceTarget(workspaces, selectedName, selectedWorkspaceDir); + }, [selectedName, selectedWorkspaceDir, workspaces]); const loading = fetchStatus === "fetching" && workspaces.length === 0; const error = fetchStatus === "error" && workspaces.length === 0 ? new Error("Failed to load workspaces") diff --git a/cmd/sgai/webapp/src/pages/InlineForkEditor.tsx b/cmd/sgai/webapp/src/pages/InlineForkEditor.tsx index 3a102c9..c8d48df 100644 --- a/cmd/sgai/webapp/src/pages/InlineForkEditor.tsx +++ b/cmd/sgai/webapp/src/pages/InlineForkEditor.tsx @@ -1,7 +1,8 @@ -import { useState, useCallback, useEffect, useRef, useTransition } from "react"; +import { useState, useCallback, useEffect, useId, useRef, useTransition } from "react"; import { useNavigate } from "react-router"; import { Button } from "@/components/ui/button"; import { Alert, AlertDescription } from "@/components/ui/alert"; +import { Label } from "@/components/ui/label"; import { MarkdownEditor } from "@/components/MarkdownEditor"; import { api, ApiError } from "@/lib/api"; import { triggerFactoryRefresh } from "@/lib/factory-state"; @@ -74,8 +75,14 @@ function clearStoredDraft(workspaceName: string): string | null { } } +function extractForkTemplateBody(content: string | null | undefined): string { + return stripFrontmatter(content ?? ""); +} + export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) { const navigate = useNavigate(); + const bodyEditorLabelId = useId(); + const bodyEditorHelpId = useId(); const [content, setContent] = useState(""); const [hasUserDraft, setHasUserDraft] = useState(false); const [isTemplateLoading, setIsTemplateLoading] = useState(false); @@ -122,7 +129,7 @@ export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) { setTemplateError(null); if (!preserveExistingContent) { setHasUserDraft(false); - setContent(result.content ?? ""); + setContent(extractForkTemplateBody(result.content)); } } catch (err) { if (cancelledRef.current) { @@ -143,7 +150,7 @@ export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) { return () => { cancelledRef.current = true; }; }, [loadTemplate, templateRequestId, workspaceName]); - const bodyText = stripFrontmatter(content).trim(); + const bodyText = content.trim(); const isBodyEmpty = bodyText.length === 0; useEffect(() => { @@ -179,7 +186,7 @@ export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) { const newValue = value ?? ""; setHasUserDraft(true); setContent(newValue); - const newBody = stripFrontmatter(newValue).trim(); + const newBody = newValue.trim(); if (newBody.length > 0) { setValidationError(null); } @@ -214,14 +221,17 @@ export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) { } } }); - }, [isBodyEmpty, workspaceName, content, navigate]); + }, [content, isBodyEmpty, navigate, workspaceName]); return (

New Task

- Write a GOAL.md for your new task. A fork will be created automatically. + Write the task body for your new fork. A fork will be created automatically. +

+

+ Root frontmatter and the inherited title are copied automatically; only the body below is submitted.

@@ -255,16 +265,22 @@ export function InlineForkEditor({ workspaceName }: InlineForkEditorProps) {
)} - +
+ +

+ Only the body you write here is submitted for the fork. +

+ +
{validationError && (

diff --git a/cmd/sgai/webapp/src/pages/ResponseModal.tsx b/cmd/sgai/webapp/src/pages/ResponseModal.tsx index b0524dc..dc661a3 100644 --- a/cmd/sgai/webapp/src/pages/ResponseModal.tsx +++ b/cmd/sgai/webapp/src/pages/ResponseModal.tsx @@ -49,6 +49,7 @@ export function ResponseModal({ error, submitting, submitError, + submitDisabledReason, selections, otherText, setOtherText, @@ -150,6 +151,10 @@ export function ResponseModal({

{submitError}

)} + {submitDisabledReason && ( +

{submitDisabledReason}

+ )} + - diff --git a/cmd/sgai/webapp/src/pages/ResponseMultiChoice.tsx b/cmd/sgai/webapp/src/pages/ResponseMultiChoice.tsx index c6fdd28..f8b27b2 100644 --- a/cmd/sgai/webapp/src/pages/ResponseMultiChoice.tsx +++ b/cmd/sgai/webapp/src/pages/ResponseMultiChoice.tsx @@ -1,5 +1,5 @@ import { useCallback } from "react"; -import { useParams, useNavigate, Link } from "react-router"; +import { useParams, useNavigate, Link, useSearchParams } from "react-router"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -12,6 +12,7 @@ import { ResponseContext } from "@/components/ResponseContext"; import { QuestionBlock } from "@/components/QuestionBlock"; import { useResponseForm } from "@/hooks/useResponseForm"; import { getRepositoryTitle } from "@/lib/repository-title"; +import { buildWorkspacePathFromName, readWorkspaceDirFromSearchParams } from "@/lib/workspace-identity"; const STORAGE_PREFIX = "sgai-response-"; @@ -37,24 +38,27 @@ function ResponseSkeleton() { export function ResponseMultiChoice() { const { name } = useParams<{ name: string }>(); + const [searchParams] = useSearchParams(); const navigate = useNavigate(); const workspaceName = name ?? ""; + const workspaceDir = readWorkspaceDirFromSearchParams(searchParams); + const workspaceProgressPath = buildWorkspacePathFromName(workspaceName, "progress", { workspaceDir }); const handleQuestionMissing = useCallback(() => { if (!workspaceName) { navigate("/", { replace: true }); return; } - navigate(`/workspaces/${encodeURIComponent(workspaceName)}/progress`, { replace: true }); - }, [navigate, workspaceName]); + navigate(workspaceProgressPath, { replace: true }); + }, [navigate, workspaceName, workspaceProgressPath]); const handleSubmitSuccess = useCallback(() => { if (!workspaceName) { navigate("/", { replace: true }); return; } - navigate(`/workspaces/${encodeURIComponent(workspaceName)}/progress`, { replace: true }); - }, [navigate, workspaceName]); + navigate(workspaceProgressPath, { replace: true }); + }, [navigate, workspaceName, workspaceProgressPath]); const { question, @@ -63,6 +67,7 @@ export function ResponseMultiChoice() { error, submitting, submitError, + submitDisabledReason, selections, otherText, setOtherText, @@ -70,6 +75,7 @@ export function ResponseMultiChoice() { handleSubmit, } = useResponseForm({ workspaceName, + workspaceDir, storagePrefix: STORAGE_PREFIX, active: true, onQuestionMissing: handleQuestionMissing, @@ -104,7 +110,7 @@ export function ResponseMultiChoice() { Failed to load question: {error.message}

← Back to workspace @@ -123,7 +129,7 @@ export function ResponseMultiChoice() {
← Back @@ -187,13 +193,19 @@ export function ResponseMultiChoice() { />
+ {submitDisabledReason && ( + + {submitDisabledReason} + + )} + {submitError && (

{submitError}

)} @@ -446,25 +481,25 @@ export function WorkspaceDetail(): JSX.Element | null { ) : ( <> {detail?.needsInput && ( - + )} {detail.continuousMode ? ( <> @@ -473,7 +508,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant={(effectiveRunning && detail.interactiveAuto) ? "default" : "outline"} onClick={handleSelfDrive} - disabled={isActionDisabled} + disabled={isStartActionDisabled} aria-pressed={effectiveRunning && detail.interactiveAuto} > Continuous Self-Drive @@ -484,7 +519,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant="outline" onClick={handleStop} - disabled={isStartStopPending} + disabled={isStopActionDisabled} > Stop @@ -497,7 +532,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant={(effectiveRunning && detail.interactiveAuto) ? "default" : "outline"} onClick={handleSelfDrive} - disabled={isActionDisabled} + disabled={isStartActionDisabled} aria-pressed={effectiveRunning && detail.interactiveAuto} > {selfDriveLabel} @@ -507,7 +542,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant={(effectiveRunning && !detail.interactiveAuto) ? "default" : "outline"} onClick={handleStart} - disabled={isActionDisabled} + disabled={isStartActionDisabled} aria-pressed={effectiveRunning && !detail.interactiveAuto} > Start @@ -518,7 +553,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant="outline" onClick={handleStop} - disabled={isStartStopPending} + disabled={isStopActionDisabled} > Stop @@ -568,7 +603,14 @@ export function WorkspaceDetail(): JSX.Element | null { type="button" size="sm" variant="outline" - onClick={() => navigate(goalEditPath)} + onClick={() => { + if (isGoalEditActionDisabled) { + return; + } + + navigate(goalEditPath); + }} + disabled={isGoalEditActionDisabled} > Edit GOAL @@ -579,7 +621,7 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant="outline" onClick={handleOpenEditor} - disabled={isEditorPending} + disabled={isEditorActionDisabled} > Open in Editor @@ -595,7 +637,7 @@ export function WorkspaceDetail(): JSX.Element | null { type="button" size="sm" variant="outline" - onClick={() => navigate(buildWorkspacePath(detail, "skills"))} + onClick={() => navigate(buildWorkspacePath(detail, "skills", { workspaceDir }))} > Skills @@ -603,7 +645,7 @@ export function WorkspaceDetail(): JSX.Element | null { type="button" size="sm" variant="outline" - onClick={() => navigate(buildWorkspacePath(detail, "snippets"))} + onClick={() => navigate(buildWorkspacePath(detail, "snippets", { workspaceDir }))} > Snippets @@ -611,7 +653,7 @@ export function WorkspaceDetail(): JSX.Element | null { type="button" size="sm" variant="outline" - onClick={() => navigate(buildWorkspacePath(detail, "agents"))} + onClick={() => navigate(buildWorkspacePath(detail, "agents", { workspaceDir }))} > Agents @@ -620,16 +662,18 @@ export function WorkspaceDetail(): JSX.Element | null { size="sm" variant={detail.pinned ? "secondary" : "outline"} onClick={handlePinToggle} - disabled={isPinPending} + disabled={isPinActionDisabled} aria-pressed={detail.pinned} > {detail.pinned ? "Unpin" : "Pin"} - + {!dirQualifiedRoute ? ( + + ) : null} )}
@@ -637,6 +681,15 @@ export function WorkspaceDetail(): JSX.Element | null { {showStatusLine && (
+ {hasMultipleActiveAgents && ( +
+ {activeAgents.map((agent) => ( + + {agent} + + ))} +
+ )} {agentModelLabel && ( @@ -647,13 +700,29 @@ export function WorkspaceDetail(): JSX.Element | null { {fullAgentModelLabel || agentModelLabel} )} + {hasMultipleActiveAgents && modelLabel && ( + + + + {modelLabel} + + + {fullAgentModelLabel || modelLabel} + + )} {statusLine && ( {statusLine} )}
- )} + )} + + {workspaceControlsDisabledReason && ( + + {workspaceControlsDisabledReason} + + )} {actionError && (

@@ -666,6 +735,7 @@ export function WorkspaceDetail(): JSX.Element | null { isRoot={detail.isRoot} hasForks={hasForks} showForkTab={showForkTab} + workspaceDir={workspaceDir} /> @@ -682,6 +752,7 @@ export function WorkspaceDetail(): JSX.Element | null { onActionClick={onActionClick} isActionRunning={isActionRunning} showForkTab={showForkTab} + dirQualifiedRoute={dirQualifiedRoute} /> )} @@ -693,6 +764,7 @@ export function WorkspaceDetail(): JSX.Element | null { activeTab={activeTab} detail={detail} showForkTab={showForkTab} + dirQualifiedRoute={dirQualifiedRoute} /> )} @@ -788,12 +860,14 @@ function TabContent({ onActionClick, isActionRunning, showForkTab, + dirQualifiedRoute, }: { activeTab: string; detail: ApiWorkspaceEntry; onActionClick?: (action: ApiActionEntry, variables: Record, targetWorkspaceName: string) => void; isActionRunning?: boolean; showForkTab: boolean; + dirQualifiedRoute: boolean; }) { switch (activeTab) { case "progress": @@ -806,6 +880,8 @@ function TabContent({ needsInput={detail.needsInput} humanMessage={detail.humanMessage} currentAgent={detail.currentAgent} + activeAgents={detail.activeAgents} + dirQualifiedRoute={dirQualifiedRoute} events={detail.events ?? []} goalContent={detail.goalContent} actions={detail.actions} @@ -837,6 +913,7 @@ function TabContent({ return (

@@ -859,9 +946,20 @@ function NoWorkspaceState({ label, goalEditPath, dir }: { label: string; goalEdi

No workspace configured for this directory.

- + {goalEditDisabledReason ? ( + + ) : ( + + )} + {goalEditDisabledReason && ( + + {goalEditDisabledReason} + + )}
); diff --git a/cmd/sgai/webapp/src/pages/__tests__/Dashboard.test.tsx b/cmd/sgai/webapp/src/pages/__tests__/Dashboard.test.tsx index c848584..575c552 100644 --- a/cmd/sgai/webapp/src/pages/__tests__/Dashboard.test.tsx +++ b/cmd/sgai/webapp/src/pages/__tests__/Dashboard.test.tsx @@ -837,9 +837,64 @@ describe("Dashboard", () => { }); }); - it("sends a basename-only delete request for duplicate-name tree rows", async () => { - const user = userEvent.setup(); + it("disambiguates tree labels when different workspaces share the same visible computed title", async () => { + mockWorkspaces = [ + createMockWorkspace({ + name: "alpha", + dir: "/tmp/first/alpha", + title: "Alpha Legacy Title", + computedTitle: "Shared Title", + }), + createMockWorkspace({ + name: "beta", + dir: "/tmp/second/beta", + title: "Beta Legacy Title", + computedTitle: "Shared Title", + }), + ]; + + renderDashboard(); + await waitFor(() => { + expect(screen.getByText("Shared Title · first")).toBeTruthy(); + expect(screen.getByText("Shared Title · second")).toBeTruthy(); + }); + + expect(screen.queryByText(/^Shared Title$/)).toBeNull(); + }); + + it("links duplicate-name tree rows through distinct dir-qualified detail routes", async () => { + mockWorkspaces = [ + createMockWorkspace({ + name: "shared-ws", + dir: "/tmp/first-parent/shared-ws", + title: "shared-ws", + computedTitle: "", + }), + createMockWorkspace({ + name: "shared-ws", + dir: "/tmp/second-parent/shared-ws", + title: "shared-ws", + computedTitle: "", + }), + ]; + + renderDashboard(); + + const firstWorkspaceDir = encodeURIComponent("/tmp/first-parent/shared-ws"); + const secondWorkspaceDir = encodeURIComponent("/tmp/second-parent/shared-ws"); + + await waitFor(() => { + const firstLink = within(getTreeRowByLabel("shared-ws · first-parent")).getByRole("link"); + const secondLink = within(getTreeRowByLabel("shared-ws · second-parent")).getByRole("link"); + + expect(firstLink.getAttribute("href")).toBe(`/workspaces/shared-ws/progress?workspaceDir=${firstWorkspaceDir}`); + expect(secondLink.getAttribute("href")).toBe(`/workspaces/shared-ws/progress?workspaceDir=${secondWorkspaceDir}`); + expect(firstLink.getAttribute("href")).not.toBe(secondLink.getAttribute("href")); + }); + }); + + it("hides destructive tree actions for duplicate-name tree rows", async () => { mockWorkspaces = [ createMockWorkspace({ name: "shared-ws", @@ -873,14 +928,8 @@ describe("Dashboard", () => { expect(screen.getByText("shared-ws · second-parent")).toBeTruthy(); }); - await user.click(screen.getByLabelText("Delete shared-ws · second-parent")); - - const dialog = await screen.findByRole("alertdialog"); - await user.click(within(dialog).getByRole("button", { name: /^Delete$/ })); - - await waitFor(() => { - expect(mockDeleteWorkspace).toHaveBeenCalledWith("shared-ws", "delete"); - }); + expect(screen.queryByLabelText("Delete shared-ws · first-parent")).toBeNull(); + expect(screen.queryByLabelText("Delete shared-ws · second-parent")).toBeNull(); }); }); diff --git a/cmd/sgai/webapp/src/pages/__tests__/InlineForkEditor.test.tsx b/cmd/sgai/webapp/src/pages/__tests__/InlineForkEditor.test.tsx index 3d50c5d..68f066f 100644 --- a/cmd/sgai/webapp/src/pages/__tests__/InlineForkEditor.test.tsx +++ b/cmd/sgai/webapp/src/pages/__tests__/InlineForkEditor.test.tsx @@ -29,19 +29,21 @@ const mockForkTemplate = mock(() => Promise.resolve({ content: "---\nflow: |\n const mockTriggerFactoryRefresh = mock(() => {}); const mockNavigate = mock(() => {}); -const mockMarkdownEditor = ({ value, onChange, disabled, placeholder, onSubmitShortcut }: { +const mockMarkdownEditor = ({ value, onChange, disabled, placeholder, onSubmitShortcut, ariaLabel }: { value: string; onChange: (v: string | undefined) => void; disabled: boolean; placeholder?: string; onSubmitShortcut?: () => void; + ariaLabel?: string; }) => ( -
-