From 14001aa259dbdb1e669f048e5de6a28d113c97be Mon Sep 17 00:00:00 2001 From: Hally Maschine Date: Wed, 25 Mar 2026 10:17:23 -0700 Subject: [PATCH] cmd/sgai: keep live workspace detail tabs responsive under load --- .gitignore | 3 +- AGENTS.md | 4 + Makefile | 9 +- browser-verification-baseline.md | 27 - cmd/sgai/delete_detach_policy_test.go | 54 +- cmd/sgai/internal_mcp_test.go | 8 + cmd/sgai/main.go | 7 +- cmd/sgai/main_test.go | 15 +- cmd/sgai/outputlog.go | 6 +- cmd/sgai/outputlog_test.go | 33 +- cmd/sgai/serve.go | 297 ++++++-- cmd/sgai/serve_api.go | 445 ++++++++---- cmd/sgai/serve_api_description.go | 2 +- cmd/sgai/serve_api_test.go | 632 +++++++++++++++--- cmd/sgai/serve_test.go | 349 +++++++++- cmd/sgai/service_adhoc.go | 6 +- cmd/sgai/service_session.go | 28 +- cmd/sgai/service_session_test.go | 340 +++++++--- cmd/sgai/service_workspace.go | 7 +- cmd/sgai/skel/.sgai/agent/general-purpose.md | 7 + .../agent/htmx-picocss-frontend-developer.md | 9 +- cmd/sgai/skel/.sgai/agent/react-developer.md | 8 +- .../browser-bug-testing-workflow/SKILL.md | 33 +- cmd/sgai/state_watcher.go | 99 +-- cmd/sgai/state_watcher_test.go | 606 ++++------------- cmd/sgai/webapp/.gitignore | 1 + cmd/sgai/webapp/dev.ts | 3 +- cmd/sgai/webapp/src/App.tsx | 21 +- cmd/sgai/webapp/src/__tests__/router.test.tsx | 15 +- cmd/sgai/webapp/src/components/ActionBar.tsx | 2 +- .../webapp/src/components/ui/alert-dialog.tsx | 2 - cmd/sgai/webapp/src/components/ui/badge.tsx | 1 - cmd/sgai/webapp/src/components/ui/card.tsx | 36 - .../webapp/src/components/ui/collapsible.tsx | 32 - .../src/components/ui/dropdown-menu.tsx | 229 ------- .../webapp/src/components/ui/scroll-area.tsx | 1 - .../webapp/src/components/ui/separator.tsx | 1 - cmd/sgai/webapp/src/components/ui/sheet.tsx | 25 - cmd/sgai/webapp/src/components/ui/sidebar.tsx | 280 -------- cmd/sgai/webapp/src/components/ui/switch.tsx | 39 -- cmd/sgai/webapp/src/components/ui/table.tsx | 28 - .../webapp/src/contexts/AppStateProvider.tsx | 94 --- cmd/sgai/webapp/src/hooks/useResponseForm.ts | 14 +- .../src/lib/__tests__/factory-state.test.ts | 229 ++++--- .../__tests__/workspace-page-state.test.ts | 259 +++++++ cmd/sgai/webapp/src/lib/factory-state.ts | 96 +-- cmd/sgai/webapp/src/lib/repository-label.ts | 27 - .../webapp/src/lib/workspace-delete-copy.ts | 42 -- cmd/sgai/webapp/src/lib/workspace-identity.ts | 2 +- .../webapp/src/lib/workspace-page-state.ts | 353 ++++++++++ cmd/sgai/webapp/src/pages/EditGoal.tsx | 4 +- cmd/sgai/webapp/src/pages/WorkspaceDetail.tsx | 265 ++++---- .../src/pages/__tests__/EditGoal.test.tsx | 85 +-- .../pages/__tests__/InlineForkEditor.test.tsx | 30 + .../__tests__/ResponseMultiChoice.test.tsx | 17 +- .../pages/__tests__/WorkspaceDetail.test.tsx | 164 ++++- cmd/sgai/webapp/src/pages/tabs/EventsTab.tsx | 79 +-- cmd/sgai/webapp/src/pages/tabs/LogTab.tsx | 34 +- .../webapp/src/pages/tabs/MessagesTab.tsx | 27 +- cmd/sgai/webapp/src/pages/tabs/SessionTab.tsx | 358 ++++++++-- .../src/pages/tabs/SpecificationTab.tsx | 68 -- .../pages/tabs/__tests__/EventsTab.test.tsx | 52 +- .../pages/tabs/__tests__/MessagesTab.test.tsx | 105 +++ .../pages/tabs/__tests__/SessionTab.test.tsx | 503 +++++++++++++- cmd/sgai/webapp/src/test-utils.ts | 109 --- cmd/sgai/webapp/src/test-utils.tsx | 42 -- cmd/sgai/webapp/src/types/index.ts | 8 +- verification-pkg-state-cleanup-dashboard.md | 27 - verification-pkg-state-console-errors.txt | 4 - 69 files changed, 4131 insertions(+), 2716 deletions(-) delete mode 100644 browser-verification-baseline.md delete mode 100644 cmd/sgai/webapp/src/components/ui/collapsible.tsx delete mode 100644 cmd/sgai/webapp/src/components/ui/dropdown-menu.tsx delete mode 100644 cmd/sgai/webapp/src/components/ui/switch.tsx delete mode 100644 cmd/sgai/webapp/src/contexts/AppStateProvider.tsx create mode 100644 cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts delete mode 100644 cmd/sgai/webapp/src/lib/repository-label.ts delete mode 100644 cmd/sgai/webapp/src/lib/workspace-delete-copy.ts create mode 100644 cmd/sgai/webapp/src/lib/workspace-page-state.ts delete mode 100644 cmd/sgai/webapp/src/pages/tabs/SpecificationTab.tsx create mode 100644 cmd/sgai/webapp/src/pages/tabs/__tests__/MessagesTab.test.tsx delete mode 100644 cmd/sgai/webapp/src/test-utils.ts delete mode 100644 cmd/sgai/webapp/src/test-utils.tsx delete mode 100644 verification-pkg-state-cleanup-dashboard.md delete mode 100644 verification-pkg-state-console-errors.txt diff --git a/.gitignore b/.gitignore index 96b19b7..d56dd1f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ verification*/ vendor/ cover*.out /*.png -opencode-src/ \ No newline at end of file +opencode-src/ +.playwright-mcp/ \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md index 2d28c76..e668316 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,10 @@ In terms of layout, UI, style, when something doesn't fit a container, use ellip CRITICAL: use playwright screenshots (and the skill to operate playwright) to verify the application is working correctly. +CRITICAL: every Playwright artifact save (`playwright_browser_take_screenshot`, `playwright_browser_snapshot`, console logs, network logs, or similar) must use a repo-relative filename under the current retrospective session's `.sgai/retrospectives//screenshots/` directory resolved from `.sgai/PROJECT_MANAGEMENT.md` frontmatter. + +CRITICAL: bare Playwright filenames like `foo.png`, `foo.md`, or `foo.txt` are forbidden because they save into the repository root and pollute the workspace. + For React/TypeScript code in cmd/sgai/webapp/, use bun for building, testing, and running scripts. Build command: `bun run build`. Dev server: `bun run dev.ts`. Tests: `bun test`. React components must use shadcn/ui components where possible. Do not create custom implementations when a shadcn component exists. Reference: https://ui.shadcn.com/docs diff --git a/Makefile b/Makefile index e09ce38..02dab6a 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,13 @@ build: webapp-build lint go build -o ./bin/sgai ./cmd/sgai -webapp-build: - cd cmd/sgai/webapp && bun install && bun run build.ts +webapp-doctor: + npx -y react-doctor@latest cmd/sgai/webapp --no-lint --verbose --fail-on warning -webapp-test: +webapp-build: + cd cmd/sgai/webapp && bun install && bun run build.ts + +webapp-test: webapp-doctor cd cmd/sgai/webapp && bun install && bun test test: webapp-test webapp-build diff --git a/browser-verification-baseline.md b/browser-verification-baseline.md deleted file mode 100644 index 141347b..0000000 --- a/browser-verification-baseline.md +++ /dev/null @@ -1,27 +0,0 @@ -- generic [ref=e2]: - - alert [ref=e3]: - - generic [ref=e4]: Enable browser notifications to get alerted when a workspace needs your input. - - generic [ref=e5]: - - button "Enable" [ref=e6] - - button "Dismiss" [ref=e7] - - main [ref=e9]: - - generic [ref=e22]: - - generic [ref=e52]: - - generic [ref=e53]: - - img "SGAI" [ref=e55] - - generic [ref=e56]: - - generic [ref=e57]: Workspaces - - generic "All factories stopped" [ref=e59]: ○ - - list [ref=e62]: - - paragraph [ref=e63]: No workspaces found. - - button "Attach external repository" [ref=e65]: - - generic [ref=e66]: "[ + ]" - - img - - generic [ref=e67]: Attach External - - button "Toggle Sidebar" [ref=e68] - - generic [ref=e43]: - - button "Toggle Sidebar" [ref=e45]: - - img - - generic [ref=e46]: Toggle Sidebar - - main [ref=e47]: - - paragraph [ref=e49]: Select a workspace to view its details \ No newline at end of file diff --git a/cmd/sgai/delete_detach_policy_test.go b/cmd/sgai/delete_detach_policy_test.go index 35f66cf..b9ad8a8 100644 --- a/cmd/sgai/delete_detach_policy_test.go +++ b/cmd/sgai/delete_detach_policy_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestHandleAPIStateIncludesRepositoryActionPolicy(t *testing.T) { +func TestHandleAPIWorkspaceListIncludesRepositoryActionPolicy(t *testing.T) { server, _ := setupTestServer(t) baseDir := t.TempDir() @@ -31,10 +31,10 @@ func TestHandleAPIStateIncludesRepositoryActionPolicy(t *testing.T) { rootDir, forkDir := setupNamedAttachedJJRootAndFork(t, server, "root-ws", "fork-ws") server.invalidateWorkspaceScanCache() - response := serveHTTP(server, http.MethodGet, "/api/v1/state", "") + response := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, response.Code) - workspaces := decodeWorkspaceStateByName(t, response.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, response.Body.Bytes()) standalone := workspaces[filepath.Base(standaloneDir)] assert.True(t, standalone.IsExternal) @@ -79,7 +79,7 @@ func TestHandleAPIStateIncludesRepositoryActionPolicy(t *testing.T) { }) } -func TestHandleAPIStateIncludesRepositoryActionPresentationMetadata(t *testing.T) { +func TestHandleAPIWorkspaceListIncludesRepositoryActionPresentationMetadata(t *testing.T) { server, _ := setupTestServer(t) baseDir := t.TempDir() @@ -87,10 +87,10 @@ func TestHandleAPIStateIncludesRepositoryActionPresentationMetadata(t *testing.T attachWorkspaceFixture(t, server, standaloneDir, workspaceStandalone) _, forkDir := setupNamedAttachedJJRootAndFork(t, server, "root-ws", "fork-ws") - response := serveHTTP(server, http.MethodGet, "/api/v1/state", "") + response := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, response.Code) - workspaces := decodeWorkspaceStateByName(t, response.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, response.Body.Bytes()) standalonePresentation := workspaces[filepath.Base(standaloneDir)].RepositoryAction.Presentation assert.Equal(t, "Detach", standalonePresentation.DetailTriggerLabel) @@ -130,7 +130,7 @@ func TestHandleAPIStateIncludesRepositoryActionPresentationMetadata(t *testing.T assert.Equal(t, "destructive", forkDeleteOperation.Tone) } -func TestHandleAPIStateClassifiesZeroChildRootFromJJMetadata(t *testing.T) { +func TestHandleAPIWorkspaceListClassifiesZeroChildRootFromJJMetadata(t *testing.T) { server, _ := setupTestServer(t) rootDir, forkDir := setupAttachedJJRootAndFork(t, server) runJJForTest(t, rootDir, "workspace", "forget", filepath.Base(forkDir)) @@ -141,10 +141,10 @@ func TestHandleAPIStateClassifiesZeroChildRootFromJJMetadata(t *testing.T) { freshServer.externalDirs[resolveSymlinks(rootDir)] = true freshServer.mu.Unlock() - response := serveHTTP(freshServer, http.MethodGet, "/api/v1/state", "") + response := serveHTTP(freshServer, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, response.Code) - workspaces := decodeWorkspaceStateByName(t, response.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, response.Body.Bytes()) root := workspaces[filepath.Base(rootDir)] assert.False(t, root.IsRoot) assert.Empty(t, root.Forks) @@ -158,7 +158,7 @@ func TestHandleAPIStateClassifiesZeroChildRootFromJJMetadata(t *testing.T) { }) } -func TestHandleAPIStateHidesRootActionWhenWorkspaceTopologyUnavailable(t *testing.T) { +func TestHandleAPIWorkspaceListHidesRootActionWhenWorkspaceTopologyUnavailable(t *testing.T) { server, _ := setupTestServer(t) rootDir, forkDir := setupAttachedJJRootAndFork(t, server) server.mu.Lock() @@ -167,10 +167,10 @@ func TestHandleAPIStateHidesRootActionWhenWorkspaceTopologyUnavailable(t *testin t.Setenv("PATH", t.TempDir()) server.invalidateWorkspaceScanCache() - response := serveHTTP(server, http.MethodGet, "/api/v1/state", "") + response := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, response.Code) - workspaces := decodeWorkspaceStateByName(t, response.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, response.Body.Bytes()) root := workspaces[filepath.Base(rootDir)] assert.True(t, root.IsRoot) assertRepositoryAction(t, &root.RepositoryAction, &repositoryActionExpectation{ @@ -368,9 +368,9 @@ func TestHandleAPIDeleteForkKeepsFactoryStateHealthy(t *testing.T) { assert.False(t, stillAttached) assert.False(t, stillPinned) - stateResponse := serveHTTP(server, http.MethodGet, "/api/v1/state", "") + stateResponse := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, stateResponse.Code) - workspaces := decodeWorkspaceStateByName(t, stateResponse.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, stateResponse.Body.Bytes()) root := workspaces[filepath.Base(rootDir)] require.NotNil(t, root) _, forkPresent := workspaces[filepath.Base(forkDir)] @@ -449,9 +449,9 @@ func TestHandleAPIDeleteWorkspaceForkDetachOperationKeepsFiles(t *testing.T) { assert.False(t, stillAttached) assert.False(t, stillPinned) - stateResponse := serveHTTP(server, http.MethodGet, "/api/v1/state", "") + stateResponse := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") require.Equal(t, http.StatusOK, stateResponse.Code) - workspaces := decodeWorkspaceStateByName(t, stateResponse.Body.Bytes()) + workspaces := decodeWorkspaceListByName(t, stateResponse.Body.Bytes()) require.NotNil(t, workspaces[filepath.Base(rootDir)]) _, forkPresent := workspaces[filepath.Base(forkDir)] assert.False(t, forkPresent) @@ -491,7 +491,7 @@ func TestHandleAPIDeleteWorkspaceRollsBackWhenPinnedPersistenceFails(t *testing. assert.True(t, pathListContains(readJSONPathList(t, filepath.Join(server.externalConfigDir, "external.json")), workspaceDir)) } -func TestBuildFullFactoryStatePrunesMissingAttachedWorkspace(t *testing.T) { +func TestBuildWorkspaceListResponsePrunesMissingAttachedWorkspace(t *testing.T) { server, _ := setupTestServer(t) server.externalConfigDir = t.TempDir() server.pinnedConfigDir = t.TempDir() @@ -508,7 +508,7 @@ func TestBuildFullFactoryStatePrunesMissingAttachedWorkspace(t *testing.T) { require.NoError(t, server.savePinnedProjects()) server.invalidateWorkspaceScanCache() - state := server.buildFullFactoryState() + state := server.buildWorkspaceListResponse() require.Len(t, state.Workspaces, 1) assert.Equal(t, filepath.Base(validDir), state.Workspaces[0].Name) @@ -522,7 +522,7 @@ func TestBuildFullFactoryStatePrunesMissingAttachedWorkspace(t *testing.T) { assert.NotContains(t, readJSONPathList(t, filepath.Join(server.pinnedConfigDir, "pinned.json")), missingCanonical) } -func TestBuildFullFactoryStateDoesNotEmitPhantomRootAfterPruningMissingAttachedRoot(t *testing.T) { +func TestBuildWorkspaceListResponseDoesNotEmitPhantomRootAfterPruningMissingAttachedRoot(t *testing.T) { server, _ := setupTestServer(t) server.externalConfigDir = t.TempDir() server.pinnedConfigDir = t.TempDir() @@ -554,7 +554,7 @@ func TestBuildFullFactoryStateDoesNotEmitPhantomRootAfterPruningMissingAttachedR require.NoError(t, server.savePinnedProjects()) server.invalidateWorkspaceScanCache() - state := server.buildFullFactoryState() + state := server.buildWorkspaceListResponse() require.Len(t, state.Workspaces, 1) assert.Equal(t, filepath.Base(forkDir), state.Workspaces[0].Name) assert.Equal(t, forkCanonical, state.Workspaces[0].Dir) @@ -581,7 +581,7 @@ func TestBuildFullFactoryStateDoesNotEmitPhantomRootAfterPruningMissingAttachedR assert.NotContains(t, readJSONPathList(t, filepath.Join(server.pinnedConfigDir, "pinned.json")), missingRootCanonical) } -func TestBuildFullFactoryStateKeepsUnreadableAttachedWorkspaceState(t *testing.T) { +func TestBuildWorkspaceListResponseKeepsUnreadableAttachedWorkspaceState(t *testing.T) { if os.Getuid() == 0 { t.Skip("skipping permission test as root") } @@ -614,7 +614,7 @@ func TestBuildFullFactoryStateKeepsUnreadableAttachedWorkspaceState(t *testing.T require.Error(t, errStat) require.False(t, os.IsNotExist(errStat)) - state := server.buildFullFactoryState() + state := server.buildWorkspaceListResponse() require.Len(t, state.Workspaces, 1) assert.Equal(t, filepath.Base(validDir), state.Workspaces[0].Name) @@ -628,7 +628,7 @@ func TestBuildFullFactoryStateKeepsUnreadableAttachedWorkspaceState(t *testing.T assert.Contains(t, readJSONPathList(t, filepath.Join(server.pinnedConfigDir, "pinned.json")), lockedCanonical) } -func TestBuildFullFactoryStateDoesNotEmitUnreadableAttachedRoot(t *testing.T) { +func TestBuildWorkspaceListResponseDoesNotEmitUnreadableAttachedRoot(t *testing.T) { if os.Getuid() == 0 { t.Skip("skipping permission test as root") } @@ -669,7 +669,7 @@ func TestBuildFullFactoryStateDoesNotEmitUnreadableAttachedRoot(t *testing.T) { require.Error(t, errStat) require.False(t, os.IsNotExist(errStat)) - state := server.buildFullFactoryState() + state := server.buildWorkspaceListResponse() require.Len(t, state.Workspaces, 1) assert.Equal(t, filepath.Base(forkDir), state.Workspaces[0].Name) assert.Equal(t, forkCanonical, state.Workspaces[0].Dir) @@ -722,13 +722,13 @@ func assertRepositoryAction(t *testing.T, repositoryAction *apiRepositoryAction, assert.ElementsMatch(t, want.allowedOperations, repositoryAction.AllowedOps) } -func decodeWorkspaceStateByName(t *testing.T, data []byte) map[string]apiWorkspaceFullState { +func decodeWorkspaceListByName(t *testing.T, data []byte) map[string]apiWorkspaceListEntry { t.Helper() - var response apiFactoryState + var response apiWorkspaceListResponse require.NoError(t, json.Unmarshal(data, &response)) - result := make(map[string]apiWorkspaceFullState, len(response.Workspaces)) + result := make(map[string]apiWorkspaceListEntry, len(response.Workspaces)) for i := range response.Workspaces { workspace := response.Workspaces[i] result[workspace.Name] = workspace diff --git a/cmd/sgai/internal_mcp_test.go b/cmd/sgai/internal_mcp_test.go index cba6534..b648a54 100644 --- a/cmd/sgai/internal_mcp_test.go +++ b/cmd/sgai/internal_mcp_test.go @@ -27,6 +27,14 @@ func TestPrintUsageOmitsInternalMCPCommand(t *testing.T) { assert.NotContains(t, output, "internal-mcp") } +func TestPrintUsageIncludesRunAndSharedConfigHelp(t *testing.T) { + output := captureStdout(t, printUsage) + assert.Contains(t, output, "sgai run [--config path] [--var key=value] ") + assert.Contains(t, output, "--shared-config-dir") + assert.Contains(t, output, "sgai run --config ./verification/sgai.json --var Name=Ada Summarize") + assert.Contains(t, output, "sgai --shared-config-dir ./verification/shared-config") +} + func TestRequiresOpencode(t *testing.T) { tests := []struct { name string diff --git a/cmd/sgai/main.go b/cmd/sgai/main.go index 09dd63d..71e1428 100644 --- a/cmd/sgai/main.go +++ b/cmd/sgai/main.go @@ -101,7 +101,8 @@ Usage: sgai run [--config path] [--var key=value] Options: - --listen-addr HTTP server listen address (default: 127.0.0.1:8080) + --listen-addr HTTP server listen address (default: 127.0.0.1:8080) + --shared-config-dir Directory for shared pinned/external workspace config Examples: sgai @@ -109,7 +110,9 @@ Examples: sgai run --config ./verification/sgai.json --var Name=Ada Summarize Run a configured action from the CLI sgai --listen-addr 0.0.0.0:8080 - Start web UI accessible externally`) + Start web UI accessible externally + sgai --shared-config-dir ./verification/shared-config + Start web UI using an isolated shared config directory`) } // runWorkflow executes the main workflow loop for a target directory. diff --git a/cmd/sgai/main_test.go b/cmd/sgai/main_test.go index 10997c0..927f743 100644 --- a/cmd/sgai/main_test.go +++ b/cmd/sgai/main_test.go @@ -142,13 +142,14 @@ func newTestMessage() state.Message { func newTestSession() *session { return &session{ - mu: sync.Mutex{}, - cancel: nil, - running: false, - outputLog: nil, - mcpCloseOnce: sync.Once{}, - mcpCloseFn: nil, - coord: nil, + mu: sync.Mutex{}, + cancel: nil, + running: false, + skipExitNotification: false, + outputLog: nil, + mcpCloseOnce: sync.Once{}, + mcpCloseFn: nil, + coord: nil, } } diff --git a/cmd/sgai/outputlog.go b/cmd/sgai/outputlog.go index 65a54dd..ed842b8 100644 --- a/cmd/sgai/outputlog.go +++ b/cmd/sgai/outputlog.go @@ -191,17 +191,15 @@ type sessionLogWriter struct { sess *session workspacePath string srv *Server - workspaceName string } -func newSessionLogWriter(sess *session, workspacePath string, srv *Server, workspaceName string) *sessionLogWriter { +func newSessionLogWriter(sess *session, workspacePath string, srv *Server) *sessionLogWriter { return &sessionLogWriter{ mu: sync.Mutex{}, partial: nil, sess: sess, workspacePath: workspacePath, srv: srv, - workspaceName: workspaceName, } } @@ -225,7 +223,7 @@ func (w *sessionLogWriter) Write(data []byte) (int, error) { func (w *sessionLogWriter) addLine(text string) { w.sess.outputLog.add(logLine{prefix: "", text: text}) - w.srv.notifyStateChange() + w.srv.notifyWorkspacePageChange(w.workspacePath) } func buildAgentOutputWriter(base io.Writer, extra ...io.Writer) io.Writer { diff --git a/cmd/sgai/outputlog_test.go b/cmd/sgai/outputlog_test.go index 0002f6b..ba109a1 100644 --- a/cmd/sgai/outputlog_test.go +++ b/cmd/sgai/outputlog_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/ucirello/sgai/pkg/state" ) func TestNewCircularLogBuffer(t *testing.T) { @@ -277,7 +278,7 @@ func TestSessionLogWriter(t *testing.T) { sess.outputLog = newCircularLogBuffer() srv, _ := setupTestServer(t) - w := newSessionLogWriter(sess, "/test", srv, "test-ws") + w := newSessionLogWriter(sess, "/test", srv) n, err := w.Write([]byte("hello world\n")) require.NoError(t, err) @@ -288,12 +289,38 @@ func TestSessionLogWriter(t *testing.T) { assert.Equal(t, "hello world", lines[0].text) } +func TestSessionLogWriterKeepsWorkspaceListCacheForLogUpdates(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "log-cache") + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stream logs" + })) + + srv.mu.Lock() + sess := srv.sessions[wsDir] + sess.outputLog = newCircularLogBuffer() + srv.mu.Unlock() + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + writer := newSessionLogWriter(sess, wsDir, srv) + writer.addLine("line 1") + + _, okList := srv.workspaceListCache.get("workspaces") + assert.True(t, okList) + assert.False(t, hasCachedWorkspacePageState(srv, wsDir)) +} + func TestSessionLogWriterMultipleLines(t *testing.T) { sess := newTestSession() sess.outputLog = newCircularLogBuffer() srv, _ := setupTestServer(t) - w := newSessionLogWriter(sess, "/test", srv, "test-ws") + w := newSessionLogWriter(sess, "/test", srv) _, _ = w.Write([]byte("line1\nline2\nline3\n")) @@ -309,7 +336,7 @@ func TestSessionLogWriterPartialLine(t *testing.T) { sess.outputLog = newCircularLogBuffer() srv, _ := setupTestServer(t) - w := newSessionLogWriter(sess, "/test", srv, "test-ws") + w := newSessionLogWriter(sess, "/test", srv) _, _ = w.Write([]byte("part")) assert.Empty(t, sess.outputLog.lines()) diff --git a/cmd/sgai/serve.go b/cmd/sgai/serve.go index 34a6b43..9917007 100644 --- a/cmd/sgai/serve.go +++ b/cmd/sgai/serve.go @@ -54,13 +54,14 @@ func stripFrontmatter(content string) string { } type session struct { - mu sync.Mutex - cancel context.CancelFunc - running bool - outputLog *circularLogBuffer - mcpCloseOnce sync.Once - mcpCloseFn func() - coord *state.Coordinator + mu sync.Mutex + cancel context.CancelFunc + running bool + skipExitNotification bool + outputLog *circularLogBuffer + mcpCloseOnce sync.Once + mcpCloseFn func() + coord *state.Coordinator } type editorOpener interface { @@ -174,9 +175,10 @@ type Server struct { classifyCache *ttlCache[string, workspaceKind] svgFlight singleflight[string, string] svgCache *ttlCache[string, string] - stateFlight singleflight[string, apiFactoryState] - stateCache *ttlCache[string, apiFactoryState] - stateGeneration uint64 + workspaceListFlight singleflight[string, apiWorkspaceListResponse] + workspaceListCache *ttlCache[string, apiWorkspaceListResponse] + workspacePageFlight singleflight[string, apiWorkspaceFullState] + workspacePageCache *ttlCache[string, apiWorkspaceFullState] goalTitleComposer func(workspacePath string, goalContent []byte) (string, error) goalTitleReadFile func(path string) ([]byte, error) @@ -222,14 +224,15 @@ func NewServer(rootDir string, paths serverPaths, editorConfig string) *Server { workspaceScanCache: newTTLCache[string, []workspaceGroup](3 * time.Second), classifyCache: newTTLCache[string, workspaceKind](5 * time.Second), svgCache: newTTLCache[string, string](10 * time.Second), - stateCache: newTTLCache[string, apiFactoryState](30 * time.Second), + workspaceListCache: newTTLCache[string, apiWorkspaceListResponse](3 * time.Second), + workspacePageCache: newTTLCache[string, apiWorkspaceFullState](3 * time.Second), promptActionRunner: nil, scriptActionRunner: nil, workspaceScanFlight: singleflight[string, []workspaceGroup]{mu: sync.Mutex{}, calls: nil}, classifyFlight: singleflight[string, workspaceKind]{mu: sync.Mutex{}, calls: nil}, svgFlight: singleflight[string, string]{mu: sync.Mutex{}, calls: nil}, - stateFlight: singleflight[string, apiFactoryState]{mu: sync.Mutex{}, calls: nil}, - stateGeneration: 0, + workspaceListFlight: singleflight[string, apiWorkspaceListResponse]{mu: sync.Mutex{}, calls: nil}, + workspacePageFlight: singleflight[string, apiWorkspaceFullState]{mu: sync.Mutex{}, calls: nil}, goalTitleComposer: defaultGoalTitleComposer, goalTitleReadFile: os.ReadFile, goalTitleRepairMu: sync.Mutex{}, @@ -267,11 +270,155 @@ func normalizeServerPaths(rootDir string, paths serverPaths) serverPaths { } func (s *Server) notifyStateChange() { - s.mu.Lock() - s.stateGeneration++ - s.mu.Unlock() - s.stateCache.delete("state") - s.signals.notify() + s.workspaceListCache.delete("workspaces") + s.workspacePageCache.deleteFunc(func(_ string) bool { return true }) + s.signals.notify(signalEvent{Name: "reload", Workspace: ""}) +} + +type workspaceListVisibleSummary struct { + Running bool + NeedsInput bool + Status string + BadgeClass string + BadgeText string + InteractiveAuto bool + CurrentAgent string + CurrentModel string + Task string + LatestProgress string + HumanMessage string +} + +func workspaceListSummaryFromState(workspacePath string, wfState *state.Workflow, running bool) workspaceListVisibleSummary { + interactiveAuto := wfState.InteractionMode == state.ModeSelfDrive || wfState.InteractionMode == state.ModeContinuous + badgeClass, badgeText := badgeStatus(wfState, running) + currentAgent := wfState.CurrentAgent + if currentAgent == "" { + currentAgent = "Unknown" + } + status := wfState.Status + if status == "" { + status = "-" + } + return workspaceListVisibleSummary{ + Running: running, + NeedsInput: wfState.NeedsHumanInput(), + Status: status, + BadgeClass: badgeClass, + BadgeText: badgeText, + InteractiveAuto: interactiveAuto, + CurrentAgent: currentAgent, + CurrentModel: resolveCurrentModel(workspacePath, wfState), + Task: wfState.Task, + LatestProgress: getLatestProgress(wfState.Progress), + HumanMessage: wfState.HumanMessage, + } +} + +func workspaceListSummaryFromEntry(entry *apiWorkspaceListEntry) workspaceListVisibleSummary { + return workspaceListVisibleSummary{ + Running: entry.Running, + NeedsInput: entry.NeedsInput, + Status: entry.Status, + BadgeClass: entry.BadgeClass, + BadgeText: entry.BadgeText, + InteractiveAuto: entry.InteractiveAuto, + CurrentAgent: entry.CurrentAgent, + CurrentModel: entry.CurrentModel, + Task: entry.Task, + LatestProgress: entry.LatestProgress, + HumanMessage: entry.HumanMessage, + } +} + +func workspaceListSummaryFromPageState(pageState *apiWorkspaceFullState) workspaceListVisibleSummary { + return workspaceListVisibleSummary{ + Running: pageState.Running, + NeedsInput: pageState.NeedsInput, + Status: pageState.Status, + BadgeClass: pageState.BadgeClass, + BadgeText: pageState.BadgeText, + InteractiveAuto: pageState.InteractiveAuto, + CurrentAgent: pageState.CurrentAgent, + CurrentModel: pageState.CurrentModel, + Task: pageState.Task, + LatestProgress: pageState.LatestProgress, + HumanMessage: pageState.HumanMessage, + } +} + +func (s *Server) workspaceListSummary(workspacePath string) (workspaceListVisibleSummary, bool) { + if cached, ok := s.workspaceListCache.get("workspaces"); ok { + idx := slices.IndexFunc(cached.Workspaces, func(entry apiWorkspaceListEntry) bool { + return sameWorkspacePath(entry.Dir, workspacePath) + }) + if idx >= 0 { + return workspaceListSummaryFromEntry(&cached.Workspaces[idx]), true + } + var zero workspaceListVisibleSummary + return zero, false + } + if cached, ok := s.workspacePageCache.get(workspacePath); ok { + return workspaceListSummaryFromPageState(&cached), true + } + var zero workspaceListVisibleSummary + return zero, false +} + +func (s *Server) workspaceListCacheNeedsInvalidation(workspacePath string, wfState *state.Workflow, running bool) bool { + next := workspaceListSummaryFromState(workspacePath, wfState, running) + current, ok := s.workspaceListSummary(workspacePath) + if !ok { + return true + } + return next != current +} + +func (s *Server) notifyWorkspacePageChange(workspacePath string) { + if workspacePath == "" { + return + } + s.workspacePageCache.delete(workspacePath) + s.signals.notify(signalEvent{Name: "workspace", Workspace: workspaceSignalPath(workspacePath)}) +} + +func workspaceSignalPath(workspacePath string) string { + if workspacePath == "" { + return "" + } + absPath, errAbs := filepath.Abs(workspacePath) + if errAbs != nil { + return resolveSymlinks(filepath.Clean(workspacePath)) + } + return resolveSymlinks(absPath) +} + +func (s *Server) notifyWorkspaceListChange(workspacePath string) { + if workspacePath == "" { + return + } + s.workspaceListCache.delete("workspaces") + s.signals.notify(signalEvent{Name: "reload", Workspace: ""}) + s.notifyWorkspacePageChange(workspacePath) +} + +func (s *Server) notifyWorkspaceChangeForState(workspacePath string, wfState *state.Workflow, running bool) { + if workspacePath == "" { + return + } + if s.workspaceListCacheNeedsInvalidation(workspacePath, wfState, running) { + s.notifyWorkspaceListChange(workspacePath) + return + } + s.notifyWorkspacePageChange(workspacePath) +} + +func (s *Server) notifyWorkspaceChangeAfterCoordinatorUpdate(workspacePath string, coord *state.Coordinator) { + if coord == nil || s.sessionRunning(workspacePath) { + return + } + wfState := coord.State() + s.notifyWorkspaceChangeForState(workspacePath, &wfState, false) } func (s *Server) validateDirectory(dir string) (string, error) { @@ -373,7 +520,10 @@ func (s *Server) startSession(workspacePath string) startSessionResult { if errCoord != nil { coord = state.NewCoordinatorEmpty(statePath(workspacePath)) } - coord.OnUpdate(s.notifyStateChange) + coord.OnUpdate(func() { + wfState := coord.State() + s.notifyWorkspaceChangeForState(workspacePath, &wfState, s.sessionRunning(workspacePath)) + }) if errUpdate := coord.UpdateState(func(wf *state.Workflow) { wf.HumanMessage = "" wf.MultiChoiceQuestion = nil @@ -405,17 +555,11 @@ func (s *Server) startSession(workspacePath string) startSessionResult { sess.cancel = cancel sess.mu.Unlock() - logWriter := newSessionLogWriter(sess, workspacePath, s, filepath.Base(workspacePath)) + logWriter := newSessionLogWriter(sess, workspacePath, s) go func() { defer func() { - sess.mcpCloseOnce.Do(mcpCloseFn) - coord.Stop() - sess.mu.Lock() - sess.running = false - sess.mu.Unlock() - s.clearEverStartedOnCompletion(workspacePath) - s.notifyStateChange() + s.finishSessionRun(workspacePath, sess, coord) }() wfState := coord.State() @@ -436,20 +580,51 @@ func (s *Server) stopSession(workspacePath string) { sess := s.sessions[workspacePath] s.mu.Unlock() + relyOnCoordinatorCancel := false if sess != nil { sess.mu.Lock() + sess.running = false + sess.skipExitNotification = true + relyOnCoordinatorCancel = sess.cancel != nil && sess.coord != nil && sess.coord.CurrentPromptToken() != "" if sess.cancel != nil { sess.cancel() } if sess.mcpCloseFn != nil { sess.mcpCloseOnce.Do(sess.mcpCloseFn) } - sess.running = false sess.mu.Unlock() } - s.resetHumanCommunication(workspacePath) - s.notifyStateChange() + if relyOnCoordinatorCancel { + return + } + if sess != nil && !s.resetHumanCommunication(workspacePath) { + s.notifyWorkspaceListChange(workspacePath) + } +} + +func (s *Server) finishSessionRun(workspacePath string, sess *session, coord *state.Coordinator) { + if sess == nil { + return + } + + sess.mu.Lock() + closeFn := sess.mcpCloseFn + skipExitNotification := sess.skipExitNotification + sess.running = false + sess.skipExitNotification = false + sess.mu.Unlock() + + if closeFn != nil { + sess.mcpCloseOnce.Do(closeFn) + } + if coord != nil { + coord.Stop() + } + s.clearEverStartedOnCompletion(workspacePath) + if !skipExitNotification { + s.notifyWorkspaceListChange(workspacePath) + } } func badgeStatus(wfState *state.Workflow, running bool) (class, text string) { @@ -481,25 +656,55 @@ func dashboardBaseURL(listenAddr string) string { return "http://" + net.JoinHostPort(host, port) } -func cmdServe(args []string) { - serveFlags := flag.NewFlagSet("serve", flag.ExitOnError) +type serveOptions struct { + listenAddr string + rootDir string + paths serverPaths +} + +func parseServeOptions(args []string, getwd func() (string, error), configHome string) (serveOptions, error) { + serveFlags := flag.NewFlagSet("serve", flag.ContinueOnError) listenAddr := serveFlags.String("listen-addr", "127.0.0.1:8080", "HTTP server listen address") - serveFlags.Parse(args) //nolint:errcheck // ExitOnError FlagSet exits on error, never returns non-nil + sharedConfigDir := serveFlags.String("shared-config-dir", "", "Directory for shared pinned/external workspace config") + if errParse := serveFlags.Parse(args); errParse != nil { + return serveOptions{}, fmt.Errorf("parse serve flags: %w", errParse) + } - var rootDir string remainingArgs := serveFlags.Args() + var rootDir string if len(remainingArgs) > 0 { rootDir = remainingArgs[0] } else { - var err error - rootDir, err = os.Getwd() - if err != nil { - log.Printf("failed to get working directory: %v", err) - return + cwd, errGetwd := getwd() + if errGetwd != nil { + return serveOptions{}, fmt.Errorf("get working directory: %w", errGetwd) } + rootDir = cwd + } + + paths := resolveServerPaths(configHome) + if *sharedConfigDir != "" { + paths = serverPaths{ + pinnedConfigDir: *sharedConfigDir, + externalConfigDir: *sharedConfigDir, + } + } + + return serveOptions{ + listenAddr: *listenAddr, + rootDir: rootDir, + paths: paths, + }, nil +} + +func cmdServe(args []string) { + options, errParse := parseServeOptions(args, os.Getwd, xdg.ConfigHome) + if errParse != nil { + log.Println("failed to parse serve flags:", errParse) + return } - listener, errListen := net.Listen("tcp4", *listenAddr) + listener, errListen := net.Listen("tcp4", options.listenAddr) if errListen != nil { log.Println("failed to listen:", errListen) return @@ -508,8 +713,7 @@ func cmdServe(args []string) { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - paths := resolveServerPaths(xdg.ConfigHome) - srv := NewServer(rootDir, paths, "") + srv := NewServer(options.rootDir, options.paths, "") srv.shutdownCtx = ctx if err := srv.loadPinnedProjects(); err != nil { log.Println("warning: failed to load pinned projects:", err) @@ -518,7 +722,6 @@ func cmdServe(args []string) { log.Println("warning: failed to load external dirs:", err) } srv.startStateWatcher() - go srv.warmStateCache() mux := http.NewServeMux() srv.registerAPIRoutes(mux) @@ -838,27 +1041,29 @@ func workspaceDagAgents(workspacePath string) []string { return flowDag.allAgents() } -func (s *Server) resetHumanCommunication(workspacePath string) { +func (s *Server) resetHumanCommunication(workspacePath string) bool { s.mu.Lock() sess := s.sessions[workspacePath] s.mu.Unlock() if sess == nil { - return + return false } sess.mu.Lock() coord := sess.coord sess.mu.Unlock() if coord == nil { - return + return false } if err := coord.UpdateState(func(wf *state.Workflow) { wf.HumanMessage = "" wf.MultiChoiceQuestion = nil }); err != nil { log.Println("failed to reset human communication state:", err) + return false } + return true } func extractSubject(body string) string { diff --git a/cmd/sgai/serve_api.go b/cmd/sgai/serve_api.go index 3c7769a..2f32474 100644 --- a/cmd/sgai/serve_api.go +++ b/cmd/sgai/serve_api.go @@ -22,8 +22,17 @@ import ( ) type signalSubscriber struct { - ch chan struct{} - done chan struct{} + ch chan signalEvent + notifyCh chan struct{} + done chan struct{} + mu sync.Mutex + pendingReload bool + pendingWorkspaces map[string]struct{} +} + +type signalEvent struct { + Name string + Workspace string } type signalBroker struct { @@ -39,12 +48,17 @@ func newSignalBroker() *signalBroker { func (b *signalBroker) subscribe() *signalSubscriber { s := &signalSubscriber{ - ch: make(chan struct{}, 1), - done: make(chan struct{}), + ch: make(chan signalEvent, 1), + notifyCh: make(chan struct{}, 1), + done: make(chan struct{}), + mu: sync.Mutex{}, + pendingReload: false, + pendingWorkspaces: make(map[string]struct{}), } b.mu.Lock() b.subscribers[s] = struct{}{} b.mu.Unlock() + go s.run() return s } @@ -55,20 +69,89 @@ func (b *signalBroker) unsubscribe(s *signalSubscriber) { close(s.done) } -func (b *signalBroker) notify() { +func (b *signalBroker) notify(event signalEvent) { b.mu.Lock() - defer b.mu.Unlock() + subscribers := make([]*signalSubscriber, 0, len(b.subscribers)) for s := range b.subscribers { + subscribers = append(subscribers, s) + } + b.mu.Unlock() + for _, s := range subscribers { + s.enqueue(event) + } +} + +func (s *signalSubscriber) run() { + for { select { - case s.ch <- struct{}{}: - default: + case <-s.done: + return + case <-s.notifyCh: + } + + for { + event, ok := s.nextPendingEvent() + if !ok { + break + } + select { + case <-s.done: + return + case s.ch <- event: + } + } + } +} + +func (s *signalSubscriber) enqueue(event signalEvent) { + select { + case <-s.done: + return + default: + } + + s.mu.Lock() + s.queueEventLocked(event) + s.mu.Unlock() + + select { + case s.notifyCh <- struct{}{}: + default: + } +} + +func (s *signalSubscriber) queueEventLocked(event signalEvent) { + switch event.Name { + case "", "reload": + s.pendingReload = true + case "workspace": + if event.Workspace != "" { + s.pendingWorkspaces[event.Workspace] = struct{}{} } + default: + s.pendingReload = true } } +func (s *signalSubscriber) nextPendingEvent() (signalEvent, bool) { + s.mu.Lock() + defer s.mu.Unlock() + if s.pendingReload { + s.pendingReload = false + return signalEvent{Name: "reload", Workspace: ""}, true + } + if len(s.pendingWorkspaces) == 0 { + return signalEvent{Name: "", Workspace: ""}, false + } + workspace := slices.Sorted(maps.Keys(s.pendingWorkspaces))[0] + delete(s.pendingWorkspaces, workspace) + return signalEvent{Name: "workspace", Workspace: workspace}, true +} + func (s *Server) registerAPIRoutes(mux *http.ServeMux) { - mux.HandleFunc("GET /api/v1/state", s.handleAPIState) mux.HandleFunc("GET /api/v1/signal", s.handleSignalStream) + mux.HandleFunc("GET /api/v1/workspaces", s.handleAPIWorkspaceList) + mux.HandleFunc("GET /api/v1/workspaces/{name}/state", s.handleAPIWorkspaceState) mux.HandleFunc("GET /api/v1/agents", s.handleAPIAgents) mux.HandleFunc("GET /api/v1/skills", s.handleAPISkills) mux.HandleFunc("GET /api/v1/skills/{name...}", s.handleAPISkillDetail) @@ -126,8 +209,18 @@ func (s *Server) handleSignalStream(w http.ResponseWriter, r *http.Request) { return case <-sub.done: return - case <-sub.ch: - if _, errWrite := fmt.Fprintf(w, "event: reload\ndata: {}\n\n"); errWrite != nil { + case event := <-sub.ch: + if event.Name == "" { + event.Name = "reload" + } + payload := struct { + Workspace string `json:"workspace,omitempty"` + }{Workspace: event.Workspace} + data, errMarshal := json.Marshal(payload) + if errMarshal != nil { + return + } + if _, errWrite := fmt.Fprintf(w, "event: %s\ndata: %s\n\n", event.Name, data); errWrite != nil { return } flusher.Flush() @@ -135,8 +228,38 @@ func (s *Server) handleSignalStream(w http.ResponseWriter, r *http.Request) { } } -type apiFactoryState struct { - Workspaces []apiWorkspaceFullState `json:"workspaces"` +type apiWorkspaceListResponse struct { + Workspaces []apiWorkspaceListEntry `json:"workspaces"` +} + +type apiWorkspaceListEntry struct { + Name string `json:"name"` + Dir string `json:"dir"` + Running bool `json:"running"` + NeedsInput bool `json:"needsInput"` + InProgress bool `json:"inProgress"` + Pinned bool `json:"pinned"` + IsRoot bool `json:"isRoot"` + IsFork bool `json:"isFork"` + IsExternal bool `json:"isExternal"` + External bool `json:"external"` + HasSGAI bool `json:"hasSgai"` + Status string `json:"status"` + BadgeClass string `json:"badgeClass"` + BadgeText string `json:"badgeText"` + HasEditedGoal bool `json:"hasEditedGoal"` + InteractiveAuto bool `json:"interactiveAuto"` + ContinuousMode bool `json:"continuousMode"` + CurrentAgent string `json:"currentAgent"` + CurrentModel string `json:"currentModel"` + Task string `json:"task"` + Title string `json:"title"` + ComputedTitle string `json:"computedTitle,omitempty"` + TotalExecTime string `json:"totalExecTime"` + LatestProgress string `json:"latestProgress"` + HumanMessage string `json:"humanMessage"` + Forks []apiForkEntry `json:"forks,omitempty"` + RepositoryAction apiRepositoryAction `json:"repositoryAction"` } type apiWorkspaceFullState struct { @@ -187,81 +310,188 @@ type apiWorkspaceFullState struct { RepositoryAction apiRepositoryAction `json:"repositoryAction"` } -func (s *Server) handleAPIState(w http.ResponseWriter, _ *http.Request) { - if cached, ok := s.stateCache.get("state"); ok { - writeJSON(w, cached) +func (s *Server) handleAPIWorkspaceList(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, s.loadWorkspaceListResponse()) +} + +var errWorkspacePageStateNotFound = errors.New("workspace not found") + +func (s *Server) handleAPIWorkspaceState(w http.ResponseWriter, r *http.Request) { + workspacePath, ok := s.resolveWorkspacePageFromPath(w, r) + if !ok { return } - factoryState, _ := s.stateFlight.do("state", func() (apiFactoryState, error) { - if cached, ok := s.stateCache.get("state"); ok { - return cached, nil + + pageState, errLoad := s.loadWorkspacePageState(workspacePath) + if errLoad != nil { + statusCode := http.StatusInternalServerError + if errors.Is(errLoad, errWorkspacePageStateNotFound) { + statusCode = http.StatusNotFound } - s.mu.Lock() - genBefore := s.stateGeneration - s.mu.Unlock() - result := s.buildFullFactoryState() - s.mu.Lock() - genAfter := s.stateGeneration - s.mu.Unlock() - if genBefore == genAfter { - s.stateCache.set("state", result) + http.Error(w, errLoad.Error(), statusCode) + return + } + + writeJSON(w, pageState) +} + +func (s *Server) resolveWorkspacePageFromPath(w http.ResponseWriter, r *http.Request) (string, bool) { + workspaceName := r.PathValue("name") + if workspaceName == "" { + http.Error(w, "workspace name is required", http.StatusBadRequest) + return "", false + } + workspacePath, statusCode, errMessage := s.resolveSingleWorkspacePath(workspaceName) + if statusCode != 0 { + http.Error(w, errMessage, statusCode) + return "", false + } + return workspacePath, true +} + +func (s *Server) loadWorkspaceListResponse() apiWorkspaceListResponse { + if cached, ok := s.workspaceListCache.get("workspaces"); ok { + return cached + } + response, _ := s.workspaceListFlight.do("workspaces", func() (apiWorkspaceListResponse, error) { + if cached, ok := s.workspaceListCache.get("workspaces"); ok { + return cached, nil } + result := s.buildWorkspaceListResponse() + s.workspaceListCache.set("workspaces", result) return result, nil }) - writeJSON(w, factoryState) + return response } -func (s *Server) warmStateCache() { - s.mu.Lock() - genBefore := s.stateGeneration - s.mu.Unlock() - result := s.buildFullFactoryState() - s.mu.Lock() - genAfter := s.stateGeneration - s.mu.Unlock() - if genBefore == genAfter { - s.stateCache.set("state", result) +func (s *Server) loadWorkspacePageState(workspacePath string) (apiWorkspaceFullState, error) { + if cached, ok := s.workspacePageCache.get(workspacePath); ok { + return cached, nil } + + pageState, errLoad := s.workspacePageFlight.do(workspacePath, func() (apiWorkspaceFullState, error) { + if cached, ok := s.workspacePageCache.get(workspacePath); ok { + return cached, nil + } + groups, errScan := s.scanWorkspaceGroups() + if errScan != nil { + return apiWorkspaceFullState{}, errScan + } + for _, ws := range workspaceInfos(groups) { + if !sameWorkspacePath(ws.Directory, workspacePath) { + continue + } + result := s.buildWorkspaceFullState(ws, groups) + s.workspacePageCache.set(workspacePath, result) + return result, nil + } + return apiWorkspaceFullState{}, errWorkspacePageStateNotFound + }) + if errLoad != nil { + return apiWorkspaceFullState{}, errLoad + } + + return pageState, nil } func (s *Server) loadWorkspaceState(dir string) state.Workflow { var emptyState state.Workflow stPath := statePath(dir) - info, errStat := os.Stat(stPath) - if errStat != nil { - return emptyState - } - if info.Size() > maxStateSizeBytes { + if _, errStat := os.Stat(stPath); errStat != nil { return emptyState } return s.workspaceCoordinator(dir).State() } -func (s *Server) buildFullFactoryState() apiFactoryState { +func (s *Server) buildWorkspaceListResponse() apiWorkspaceListResponse { groups, errScan := s.scanWorkspaceGroups() if errScan != nil { - return apiFactoryState{Workspaces: nil} - } - - var allWorkspaces []workspaceInfo - for _, grp := range groups { - allWorkspaces = append(allWorkspaces, grp.Root) - allWorkspaces = append(allWorkspaces, grp.Forks...) + return apiWorkspaceListResponse{Workspaces: nil} } - workspaces := make([]apiWorkspaceFullState, len(allWorkspaces)) + allWorkspaces := workspaceInfos(groups) + workspaces := make([]apiWorkspaceListEntry, len(allWorkspaces)) var wg sync.WaitGroup for i, ws := range allWorkspaces { wg.Go(func() { - workspaces[i] = s.buildWorkspaceFullState(ws, groups) + workspaces[i] = s.buildWorkspaceListEntry(ws, groups) }) } wg.Wait() - return apiFactoryState{Workspaces: workspaces} + return apiWorkspaceListResponse{Workspaces: workspaces} } -const maxStateSizeBytes = 10 * 1024 * 1024 +func workspaceHasEditedGoal(dir string) bool { + data, errRead := os.ReadFile(filepath.Join(dir, "GOAL.md")) + if errRead != nil { + return false + } + body := extractBody(data) + return strings.TrimSpace(string(body)) != "" +} + +func (s *Server) buildWorkspaceListEntry(ws workspaceInfo, groups []workspaceGroup) apiWorkspaceListEntry { + wfState := s.loadWorkspaceState(ws.Directory) + kind := s.classifyWorkspaceCached(ws.Directory) + + interactiveAuto := wfState.InteractionMode == state.ModeSelfDrive || wfState.InteractionMode == state.ModeContinuous + badgeClass, badgeText := badgeStatus(&wfState, ws.Running) + needsInput := wfState.NeedsHumanInput() + + currentAgent := wfState.CurrentAgent + if currentAgent == "" { + currentAgent = "Unknown" + } + + status := wfState.Status + if status == "" { + status = "-" + } + + titleState := goalTitleStateFromPath(ws.Directory, ws.DirName) + if titleState.NeedsRepair { + s.enqueueGoalTitleRepair(ws.Directory) + } + + repositoryAction := s.workspaceActionPolicy(ws.Directory) + + result := apiWorkspaceListEntry{ + Name: ws.DirName, + Dir: ws.Directory, + Running: ws.Running, + NeedsInput: needsInput, + InProgress: ws.InProgress, + Pinned: ws.Pinned, + IsRoot: ws.IsRoot, + IsFork: kind == workspaceFork, + IsExternal: ws.External, + External: ws.External, + HasSGAI: ws.HasWorkspace, + Status: status, + BadgeClass: badgeClass, + BadgeText: badgeText, + HasEditedGoal: workspaceHasEditedGoal(ws.Directory), + InteractiveAuto: interactiveAuto, + ContinuousMode: readContinuousModePrompt(ws.Directory) != "", + CurrentAgent: currentAgent, + CurrentModel: resolveCurrentModel(ws.Directory, &wfState), + Task: wfState.Task, + Title: titleState.Title, + ComputedTitle: workspaceComputedTitle(ws, groups, titleState), + TotalExecTime: calculateTotalExecutionTime(wfState.AgentSequence, ws.Running, getLastActivityTime(wfState.Progress)), + LatestProgress: getLatestProgress(wfState.Progress), + HumanMessage: wfState.HumanMessage, + Forks: nil, + RepositoryAction: repositoryAction.api(ws.DirName), + } + + if ws.IsRoot { + result.Forks = s.collectForksForAPIFromGroups(ws.Directory, groups) + } + + return result +} func (s *Server) buildWorkspaceFullState(ws workspaceInfo, groups []workspaceGroup) apiWorkspaceFullState { wfState := s.loadWorkspaceState(ws.Directory) @@ -287,11 +517,7 @@ func (s *Server) buildWorkspaceFullState(ws workspaceInfo, groups []workspaceGro s.enqueueGoalTitleRepair(ws.Directory) } - hasEditedGoal := false - if data, errRead := os.ReadFile(filepath.Join(ws.Directory, "GOAL.md")); errRead == nil { - body := extractBody(data) - hasEditedGoal = strings.TrimSpace(string(body)) != "" - } + hasEditedGoal := workspaceHasEditedGoal(ws.Directory) agentSeq := convertAgentSequence( prepareAgentSequenceDisplay(wfState.AgentSequence, ws.Running, getLastActivityTime(wfState.Progress), ws.Directory), @@ -1144,11 +1370,11 @@ func (s *Server) handleAPIRespond(w http.ResponseWriter, r *http.Request) { coord := s.sessionCoordinator(workspacePath) if coord == nil { - http.Error(w, "no pending question", http.StatusConflict) + http.Error(w, errNoPendingQuestion.Error(), http.StatusConflict) return } - s.handleRespondViaCoordinator(w, coord, req) + s.handleRespondViaCoordinator(w, workspacePath, coord, req) } func (s *Server) sessionCoordinator(workspacePath string) *state.Coordinator { @@ -1163,42 +1389,21 @@ func (s *Server) sessionCoordinator(workspacePath string) *state.Coordinator { return sess.coord } -func (s *Server) handleRespondViaCoordinator(w http.ResponseWriter, coord *state.Coordinator, req apiRespondRequest) { - wfState := coord.State() - - if !wfState.NeedsHumanInput() { - http.Error(w, "no pending question", http.StatusConflict) - return - } - - responseText := buildAPIResponseText(req) - if responseText == "" { - http.Error(w, "response cannot be empty", http.StatusBadRequest) - return - } - - if !coord.RespondIfCurrent(req.PromptToken, responseText) { - http.Error(w, "question not available", http.StatusConflict) - return - } - - if wfState.MultiChoiceQuestion != nil && wfState.MultiChoiceQuestion.IsWorkGate { - approvedViaSelection := slices.Contains(req.SelectedChoices, workGateApprovalText) - if approvedViaSelection { - if err := coord.UpdateState(func(wf *state.Workflow) { - if wf.InteractionMode == state.ModeBrainstorming { - wf.InteractionMode = state.ModeBuilding - } - }); err != nil { - http.Error(w, "failed to save work gate approval", http.StatusInternalServerError) - return - } +func (s *Server) handleRespondViaCoordinator(w http.ResponseWriter, workspacePath string, coord *state.Coordinator, req apiRespondRequest) { + result, errRespond := s.respondViaCoordinatorService(workspacePath, coord, req) + if errRespond != nil { + statusCode := http.StatusInternalServerError + switch { + case errors.Is(errRespond, errNoPendingQuestion), errors.Is(errRespond, errQuestionNotAvailable): + statusCode = http.StatusConflict + case errors.Is(errRespond, errResponseCannotBeEmpty): + statusCode = http.StatusBadRequest } + http.Error(w, errRespond.Error(), statusCode) + return } - s.notifyStateChange() - - writeJSON(w, apiRespondResponse{Success: true, Message: "response submitted"}) + writeJSON(w, apiRespondResponse(result)) } func buildAPIResponseText(req apiRespondRequest) string { @@ -1711,36 +1916,17 @@ func (s *Server) handleAPISteer(w http.ResponseWriter, r *http.Request) { return } - if strings.TrimSpace(req.Message) == "" { - http.Error(w, "message cannot be empty", http.StatusBadRequest) - return - } - - coord := s.workspaceCoordinator(workspacePath) - steerBody := "Re-steering instruction: " + strings.TrimSpace(req.Message) - steerCreatedAt := time.Now().UTC().Format(time.RFC3339) - - if errUpdate := coord.UpdateState(func(wf *state.Workflow) { - newMsg := state.Message{ - ID: nextMessageID(wf.Messages), - FromAgent: "Human Partner", - ToAgent: "coordinator", - Body: steerBody, - Read: false, - ReadAt: "", - ReadBy: "", - CreatedAt: steerCreatedAt, + result, errSteer := s.steerService(workspacePath, req.Message) + if errSteer != nil { + statusCode := http.StatusInternalServerError + if errors.Is(errSteer, errSteerMessageEmpty) { + statusCode = http.StatusBadRequest } - insertIdx := findSteerInsertPosition(wf.Messages) - wf.Messages = slices.Insert(wf.Messages, insertIdx, newMsg) - }); errUpdate != nil { - http.Error(w, "failed to save state", http.StatusInternalServerError) + http.Error(w, errSteer.Error(), statusCode) return } - s.notifyStateChange() - - writeJSON(w, apiSteerResponse{Success: true, Message: "steering instruction added"}) + writeJSON(w, apiSteerResponse(result)) } func findSteerInsertPosition(messages []state.Message) int { @@ -1763,20 +1949,13 @@ func (s *Server) handleAPITogglePin(w http.ResponseWriter, r *http.Request) { return } - if errToggle := s.togglePin(workspacePath); errToggle != nil { - http.Error(w, "failed to toggle pin", http.StatusInternalServerError) + result, errToggle := s.togglePinService(workspacePath) + if errToggle != nil { + http.Error(w, errToggle.Error(), http.StatusInternalServerError) return } - s.invalidateWorkspaceScanCache() - s.notifyStateChange() - - pinned := s.isPinned(workspacePath) - - writeJSON(w, apiTogglePinResponse{ - Pinned: pinned, - Message: "pin toggled", - }) + writeJSON(w, apiTogglePinResponse(result)) } type apiOpenEditorResponse struct { diff --git a/cmd/sgai/serve_api_description.go b/cmd/sgai/serve_api_description.go index 7a1520f..0741d70 100644 --- a/cmd/sgai/serve_api_description.go +++ b/cmd/sgai/serve_api_description.go @@ -308,6 +308,6 @@ func (s *Server) repairGoalTitle(workspacePath string) error { } return fmt.Errorf("write GOAL.md: %w", errWrite) } - s.notifyStateChange() + s.notifyWorkspaceListChange(workspacePath) return nil } diff --git a/cmd/sgai/serve_api_test.go b/cmd/sgai/serve_api_test.go index 1c6eecb..31ff4ba 100644 --- a/cmd/sgai/serve_api_test.go +++ b/cmd/sgai/serve_api_test.go @@ -1,9 +1,11 @@ package main import ( + "bytes" "context" "encoding/json" "errors" + "fmt" "io/fs" "net/http" "net/http/httptest" @@ -14,6 +16,7 @@ import ( "sync" "testing" "testing/fstest" + "testing/synctest" "time" "github.com/adrg/xdg" @@ -29,6 +32,50 @@ func newTestServeSession(coord *state.Coordinator, running bool) *session { return sess } +type lockedResponseRecorder struct { + header http.Header + body bytes.Buffer + code int + mu sync.Mutex +} + +func newLockedResponseRecorder() *lockedResponseRecorder { + var recorder lockedResponseRecorder + recorder.header = make(http.Header) + return &recorder +} + +func (w *lockedResponseRecorder) Header() http.Header { + return w.header +} + +func (w *lockedResponseRecorder) WriteHeader(statusCode int) { + if w.code == 0 { + w.code = statusCode + } +} + +func (w *lockedResponseRecorder) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + if w.code == 0 { + w.code = http.StatusOK + } + n, errWrite := w.body.Write(p) + if errWrite != nil { + return n, fmt.Errorf("write locked response body: %w", errWrite) + } + return n, nil +} + +func (w *lockedResponseRecorder) Flush() {} + +func (w *lockedResponseRecorder) BodyString() string { + w.mu.Lock() + defer w.mu.Unlock() + return w.body.String() +} + func workflowWith(update func(*state.Workflow)) state.Workflow { return updated(newTestWorkflow(), update) } @@ -137,10 +184,29 @@ func writeWorkflowStateToDisk(t *testing.T, wsDir string, wf *state.Workflow) { require.NoError(t, errCoord) } +func writeWorkflowStateWithLaterModTime(t *testing.T, wsDir string, wf *state.Workflow) { + t.Helper() + + info, errStat := os.Stat(statePath(wsDir)) + require.NoError(t, errStat) + + writeWorkflowStateToDisk(t, wsDir, wf) + setLaterFileModTime(t, statePath(wsDir), info.ModTime().Add(time.Second)) +} + func startWaitingSessionQuestion(t *testing.T, srv *Server, wsDir string, question *state.MultiChoiceQuestion, humanMessage string) (*state.Coordinator, <-chan error, context.CancelFunc) { t.Helper() statePath := filepath.Join(wsDir, ".sgai", "state.json") coord := state.NewCoordinatorEmpty(statePath) + ready := make(chan struct{}, 1) + coord.OnUpdate(func() { + if coord.State().NeedsHumanInput() { + select { + case ready <- struct{}{}: + default: + } + } + }) srv.mu.Lock() srv.sessions[wsDir] = newTestServeSession(coord, true) srv.mu.Unlock() @@ -151,9 +217,7 @@ func startWaitingSessionQuestion(t *testing.T, srv *Server, wsDir string, questi _, err := coord.AskAndWait(ctx, question, humanMessage) errCh <- err }() - require.Eventually(t, func() bool { - return coord.State().NeedsHumanInput() - }, time.Second, 10*time.Millisecond) + <-ready return coord, errCh, cancel } @@ -165,7 +229,7 @@ func TestIsAPIRoute(t *testing.T) { }{ { name: "apiRoute", - urlPath: "/api/v1/state", + urlPath: "/api/v1/workspaces", expected: true, }, { @@ -632,11 +696,6 @@ func TestCoordinatorModelFromWorkspace(t *testing.T) { } } -func TestWarmStateCache(t *testing.T) { - server, _ := setupTestServer(t) - server.warmStateCache() -} - func TestSessionCoordinator(t *testing.T) { srv, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, srv, rootDir, "sess-coord-ws") @@ -654,7 +713,7 @@ func TestWriteJSONResponse(t *testing.T) { srv, rootDir := setupTestServer(t) _ = setupTestWorkspace(t, srv, rootDir, "json-ws") - w := serveHTTP(srv, "GET", "/api/v1/state", "") + w := serveHTTP(srv, "GET", "/api/v1/workspaces", "") assert.Equal(t, http.StatusOK, w.Code) var resp json.RawMessage @@ -674,7 +733,7 @@ func TestConvertSnippetLanguagesEmpty(t *testing.T) { } func TestIsAPIRouteVariants(t *testing.T) { - assert.True(t, isAPIRoute("/api/v1/state")) + assert.True(t, isAPIRoute("/api/v1/workspaces")) assert.True(t, isAPIRoute("/api/v1/agents")) assert.False(t, isAPIRoute("/")) assert.False(t, isAPIRoute("/index.html")) @@ -684,7 +743,7 @@ func TestIsStaticAssetVariants(t *testing.T) { assert.True(t, isStaticAsset("/assets/main.js")) assert.True(t, isStaticAsset("/assets/style.css")) assert.True(t, isStaticAsset("/favicon.ico")) - assert.False(t, isStaticAsset("/api/v1/state")) + assert.False(t, isStaticAsset("/api/v1/workspaces")) assert.False(t, isStaticAsset("/")) } @@ -916,22 +975,14 @@ func TestConvertActionsForAPIEmpty(t *testing.T) { assert.Empty(t, result) } -func TestBuildFullFactoryStateWithMultipleWorkspaces(t *testing.T) { +func TestBuildWorkspaceListResponseWithMultipleWorkspaces(t *testing.T) { server, rootDir := setupTestServer(t) setupTestWorkspace(t, server, rootDir, "ws1") setupTestWorkspace(t, server, rootDir, "ws2") - result := server.buildFullFactoryState() + result := server.buildWorkspaceListResponse() assert.GreaterOrEqual(t, len(result.Workspaces), 2) } -func TestWarmStateCacheFillsCache(t *testing.T) { - server, rootDir := setupTestServer(t) - setupTestWorkspace(t, server, rootDir, "test-ws") - server.warmStateCache() - _, ok := server.stateCache.get("state") - assert.True(t, ok) -} - func TestReadNewestForkGoalEmptyList(t *testing.T) { result := readNewestForkGoal(nil) assert.Empty(t, result) @@ -1052,14 +1103,13 @@ func TestQuestionTypeWorkGateFlag(t *testing.T) { assert.Equal(t, "work-gate", questionType(&wf)) } -func TestLoadWorkspaceStateLargeFileReturnsEmpty(t *testing.T) { +func TestLoadWorkspaceStateInvalidJSONReturnsWorkingFallback(t *testing.T) { server, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, server, rootDir, "test-ws-large") sp := filepath.Join(wsDir, ".sgai", "state.json") - largeContent := strings.Repeat("x", maxStateSizeBytes+1) - require.NoError(t, os.WriteFile(sp, []byte(largeContent), 0o644)) + require.NoError(t, os.WriteFile(sp, []byte(`{invalid}`), 0o644)) result := server.loadWorkspaceState(wsDir) - assert.Empty(t, result.Status) + assert.Equal(t, state.StatusWorking, result.Status) } func TestLoadWorkspaceStateNoFileReturnsEmpty(t *testing.T) { @@ -1499,18 +1549,34 @@ func TestLoadWorkspaceState(t *testing.T) { assert.Equal(t, "test-agent", wf.CurrentAgent) }) - t.Run("oversizedState", func(t *testing.T) { + t.Run("largeValidState", func(t *testing.T) { rootDir := t.TempDir() server := NewServer(rootDir, newTestServerPaths(), "") workDir := filepath.Join(rootDir, "ws") sgaiDir := filepath.Join(workDir, ".sgai") require.NoError(t, os.MkdirAll(sgaiDir, 0o755)) - bigContent := strings.Repeat("x", 11*1024*1024) - require.NoError(t, os.WriteFile(filepath.Join(sgaiDir, "state.json"), []byte(bigContent), 0o644)) + stateFile := filepath.Join(sgaiDir, "state.json") + _, err := state.NewCoordinatorWith(stateFile, workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "builder" + workflow.Messages = []state.Message{{ + ID: 1, + FromAgent: "coordinator", + ToAgent: "builder", + Body: strings.Repeat("x", 11*1024*1024), + Read: false, + ReadAt: "", + ReadBy: "", + CreatedAt: "", + }} + })) + require.NoError(t, err) wf := server.loadWorkspaceState(workDir) - assert.Empty(t, wf.Status) + assert.Equal(t, state.StatusWorking, wf.Status) + assert.Equal(t, "builder", wf.CurrentAgent) + require.Len(t, wf.Messages, 1) }) } @@ -1572,17 +1638,114 @@ func serveHTTP(server *Server, method, path, body string) *httptest.ResponseReco return w } -func TestHandleAPIState(t *testing.T) { +func TestHandleAPIWorkspaceListRoute(t *testing.T) { server, rootDir := setupTestServer(t) setupTestWorkspace(t, server, rootDir, "test-ws") goalContent := "---\nflow: |\n \"a\" -> \"b\"\n---\n# Test" require.NoError(t, os.WriteFile(filepath.Join(rootDir, "test-ws", "GOAL.md"), []byte(goalContent), 0o644)) - w := serveHTTP(server, "GET", "/api/v1/state", "") + w := serveHTTP(server, "GET", "/api/v1/workspaces", "") assert.Equal(t, http.StatusOK, w.Code) assert.Contains(t, w.Header().Get("Content-Type"), "application/json") } +func TestHandleAPIWorkspaceList(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "test-ws") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\nflow: |\n \"a\" -> \"b\"\n---\n# Test"), 0o644)) + _, errCoord := state.NewCoordinatorWith(filepath.Join(wsDir, ".sgai", "state.json"), workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + workflow.Progress = []state.ProgressEntry{{Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 1"}} + workflow.Messages = []state.Message{{ID: 1, FromAgent: "a", ToAgent: "b", Body: "heavy message payload", Read: false, ReadAt: "", ReadBy: "", CreatedAt: ""}} + })) + require.NoError(t, errCoord) + + w := serveHTTP(server, http.MethodGet, "/api/v1/workspaces", "") + assert.Equal(t, http.StatusOK, w.Code) + + var resp struct { + Workspaces []map[string]json.RawMessage `json:"workspaces"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + require.Len(t, resp.Workspaces, 1) + workspace := resp.Workspaces[0] + + var name string + require.NoError(t, json.Unmarshal(workspace["name"], &name)) + assert.Equal(t, "test-ws", name) + assert.NotContains(t, workspace, "messages") + assert.NotContains(t, workspace, "events") + assert.NotContains(t, workspace, "log") + assert.NotContains(t, workspace, "goalContent") + assert.NotContains(t, workspace, "pmContent") +} + +func TestHandleAPIWorkspaceState(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "test-ws") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\nflow: |\n \"a\" -> \"b\"\n---\n# Test"), 0o644)) + _, errCoord := state.NewCoordinatorWith(filepath.Join(wsDir, ".sgai", "state.json"), workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + workflow.Progress = []state.ProgressEntry{{Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 1"}} + workflow.Messages = []state.Message{{ID: 1, FromAgent: "a", ToAgent: "b", Body: "hello", Read: false, ReadAt: "", ReadBy: "", CreatedAt: ""}} + })) + require.NoError(t, errCoord) + + sess := newTestServeSession(nil, true) + sess.outputLog = newCircularLogBuffer() + sess.outputLog.add(logLine{prefix: "", text: "log line"}) + server.mu.Lock() + server.sessions[wsDir] = sess + server.mu.Unlock() + + w := serveHTTP(server, http.MethodGet, "/api/v1/workspaces/test-ws/state", "") + assert.Equal(t, http.StatusOK, w.Code) + + var resp apiWorkspaceFullState + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(t, "test-ws", resp.Name) + require.Len(t, resp.Messages, 1) + require.Len(t, resp.Events, 1) + require.Len(t, resp.Log, 1) +} + +func TestHandleAPIWorkspaceStateRejectsAmbiguousBasenameEvenWithWorkspaceHeader(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("# First Goal"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(secondDir, "GOAL.md"), []byte("# Second Goal"), 0o644)) + + server.mu.Lock() + server.externalDirs[resolveSymlinks(firstDir)] = true + server.externalDirs[resolveSymlinks(secondDir)] = true + server.mu.Unlock() + server.invalidateWorkspaceScanCache() + + mux := http.NewServeMux() + server.registerAPIRoutes(mux) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/workspaces/shared-ws/state", http.NoBody) + req.Header.Set("X-Sgai-Workspace-Dir", workspaceSignalPath(secondDir)) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusConflict, w.Code) + assert.Contains(t, w.Body.String(), "workspace name is ambiguous") +} + +func TestLoadWorkspacePageStateMissingWorkspace(t *testing.T) { + server, _ := setupTestServer(t) + + _, err := server.loadWorkspacePageState("/missing/workspace") + require.Error(t, err) + assert.Contains(t, err.Error(), "workspace not found") +} + func TestHandleAPIAgents(t *testing.T) { server, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, server, rootDir, "test-ws") @@ -2238,7 +2401,7 @@ func TestSPAMiddlewareAPIRoutes(t *testing.T) { server, rootDir := setupTestServer(t) setupTestWorkspace(t, server, rootDir, "test-ws") - w := serveHTTP(server, "GET", "/api/v1/state", "") + w := serveHTTP(server, "GET", "/api/v1/workspaces", "") assert.Equal(t, http.StatusOK, w.Code) } @@ -2750,7 +2913,7 @@ func TestBuildWorkspaceFullStateWithTodos(t *testing.T) { assert.Len(t, result.ProjectTodos, 1) } -func TestHandleAPIStateFullIntegration(t *testing.T) { +func TestHandleAPIWorkspaceStateFullIntegration(t *testing.T) { srv, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, srv, rootDir, "full-int") require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\nflow: |\n digraph G {\n \"coordinator\" -> \"builder\"\n \"builder\" -> \"reviewer\"\n }\nmodels:\n coordinator: anthropic/claude-opus-4-6\n---\n# Full Integration\n\nBuild a comprehensive test suite."), 0o644)) @@ -2781,19 +2944,16 @@ func TestHandleAPIStateFullIntegration(t *testing.T) { })) require.NoError(t, errCoord) - w := serveHTTP(srv, "GET", "/api/v1/state", "") + w := serveHTTP(srv, "GET", "/api/v1/workspaces/full-int/state", "") assert.Equal(t, http.StatusOK, w.Code) - var resp apiFactoryState - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - require.NotEmpty(t, resp.Workspaces) - - ws := resp.Workspaces[0] + var ws apiWorkspaceFullState + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &ws)) assert.Equal(t, "full-int", ws.Name) assert.Equal(t, string(state.StatusComplete), ws.Status) } -func TestHandleAPIStateWithPendingQuestion(t *testing.T) { +func TestHandleAPIWorkspaceStateWithPendingQuestion(t *testing.T) { srv, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, srv, rootDir, "pq-int") require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\n---\n# Goal"), 0o644)) @@ -2817,21 +2977,18 @@ func TestHandleAPIStateWithPendingQuestion(t *testing.T) { }) })) - w := serveHTTP(srv, "GET", "/api/v1/state", "") + w := serveHTTP(srv, "GET", "/api/v1/workspaces/pq-int/state", "") assert.Equal(t, http.StatusOK, w.Code) - var resp apiFactoryState - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - require.NotEmpty(t, resp.Workspaces) - - ws := resp.Workspaces[0] + var ws apiWorkspaceFullState + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &ws)) assert.True(t, ws.NeedsInput) assert.NotNil(t, ws.PendingQuestion) assert.Len(t, ws.PendingQuestion.Questions, 2) assert.Equal(t, "coordinator", ws.PendingQuestion.AgentName) } -func TestHandleAPIStatePendingQuestionUsesPromptToken(t *testing.T) { +func TestHandleAPIWorkspaceStatePendingQuestionUsesPromptToken(t *testing.T) { srv, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, srv, rootDir, "pq-token") require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\n---\n# Goal"), 0o644)) @@ -2847,25 +3004,19 @@ func TestHandleAPIStatePendingQuestionUsesPromptToken(t *testing.T) { defer cancel() promptToken := waitForSessionPromptToken(t, coord) - w := serveHTTP(srv, "GET", "/api/v1/state", "") + w := serveHTTP(srv, "GET", "/api/v1/workspaces/pq-token/state", "") assert.Equal(t, http.StatusOK, w.Code) - var resp apiFactoryState - require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) - require.NotEmpty(t, resp.Workspaces) - - workspace := resp.Workspaces[0] + var workspace apiWorkspaceFullState + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &workspace)) require.NotNil(t, workspace.PendingQuestion) assert.Equal(t, promptToken, workspace.PendingQuestion.PromptToken) var rawResp struct { - Workspaces []struct { - PendingQuestion map[string]json.RawMessage `json:"pendingQuestion"` - } `json:"workspaces"` + PendingQuestion map[string]json.RawMessage `json:"pendingQuestion"` } require.NoError(t, json.Unmarshal(w.Body.Bytes(), &rawResp)) - require.NotEmpty(t, rawResp.Workspaces) - _, hasLegacyField := rawResp.Workspaces[0].PendingQuestion["questionId"] + _, hasLegacyField := rawResp.PendingQuestion["questionId"] assert.False(t, hasLegacyField) cancel() @@ -2916,15 +3067,15 @@ func TestRespondViaCoordinatorWithoutActiveToolCall(t *testing.T) { assert.Equal(t, http.StatusConflict, w.Code) } -func TestHandleAPIStateWithCaching(t *testing.T) { +func TestHandleAPIWorkspaceStateWithCaching(t *testing.T) { srv, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, srv, rootDir, "cache-ws") require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\n---\n# Goal"), 0o644)) - w1 := serveHTTP(srv, "GET", "/api/v1/state", "") + w1 := serveHTTP(srv, "GET", "/api/v1/workspaces/cache-ws/state", "") assert.Equal(t, http.StatusOK, w1.Code) - w2 := serveHTTP(srv, "GET", "/api/v1/state", "") + w2 := serveHTTP(srv, "GET", "/api/v1/workspaces/cache-ws/state", "") assert.Equal(t, http.StatusOK, w2.Code) } @@ -3246,6 +3397,9 @@ func TestBuildFullFactoryStateRepairsMissingTitlesSequentially(t *testing.T) { var mu sync.Mutex currentConcurrent := 0 maxConcurrent := 0 + started := make(chan string, 2) + finished := make(chan string, 2) + release := make(chan struct{}, 2) server.goalTitleComposer = func(workspacePath string, _ []byte) (string, error) { mu.Lock() currentConcurrent++ @@ -3254,18 +3408,63 @@ func TestBuildFullFactoryStateRepairsMissingTitlesSequentially(t *testing.T) { } mu.Unlock() - time.Sleep(20 * time.Millisecond) + started <- filepath.Base(workspacePath) + <-release mu.Lock() currentConcurrent-- mu.Unlock() + finished <- filepath.Base(workspacePath) return strings.TrimSuffix(filepath.Base(workspacePath), "-ws") + " repaired", nil } - result := server.buildFullFactoryState() + result := server.buildWorkspaceListResponse() require.Len(t, result.Workspaces, 2) + var firstStarted string + require.Eventually(t, func() bool { + select { + case firstStarted = <-started: + return true + default: + return false + } + }, time.Second, 10*time.Millisecond) + + select { + case secondStarted := <-started: + t.Fatalf("second repair started before first was released: %s", secondStarted) + default: + } + + release <- struct{}{} + + var secondStarted string + require.Eventually(t, func() bool { + select { + case secondStarted = <-started: + return true + default: + return false + } + }, time.Second, 10*time.Millisecond) + assert.NotEqual(t, firstStarted, secondStarted) + + release <- struct{}{} + + finishedCount := 0 + require.Eventually(t, func() bool { + for { + select { + case <-finished: + finishedCount++ + default: + return finishedCount == 2 + } + } + }, time.Second, 10*time.Millisecond) + require.Eventually(t, func() bool { firstData, errFirst := os.ReadFile(filepath.Join(firstDir, "GOAL.md")) secondData, errSecond := os.ReadFile(filepath.Join(secondDir, "GOAL.md")) @@ -4071,7 +4270,7 @@ func TestSPAMiddlewareAPIRoute(t *testing.T) { srv.registerAPIRoutes(mux) handler := srv.spaMiddleware(mux) - req := httptest.NewRequest(http.MethodGet, "/api/v1/state", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/api/v1/workspaces", http.NoBody) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -4295,7 +4494,7 @@ func TestHandleRespondViaCoordinator(t *testing.T) { require.NoError(t, errCoord) w := httptest.NewRecorder() - srv.handleRespondViaCoordinator(w, coord, respondRequestWith(func(request *apiRespondRequest) { + srv.handleRespondViaCoordinator(w, dir, coord, respondRequestWith(func(request *apiRespondRequest) { request.Answer = "yes" })) assert.Equal(t, http.StatusConflict, w.Code) @@ -4321,7 +4520,7 @@ func TestHandleRespondViaCoordinator(t *testing.T) { require.NoError(t, errCoord) w := httptest.NewRecorder() - srv.handleRespondViaCoordinator(w, coord, respondRequestWith(func(request *apiRespondRequest) { + srv.handleRespondViaCoordinator(w, dir, coord, respondRequestWith(func(request *apiRespondRequest) { request.Answer = "yes" })) assert.Equal(t, http.StatusConflict, w.Code) @@ -4347,7 +4546,7 @@ func TestHandleRespondViaCoordinator(t *testing.T) { require.NoError(t, errCoord) w := httptest.NewRecorder() - srv.handleRespondViaCoordinator(w, coord, newTestAPIRespondRequest()) + srv.handleRespondViaCoordinator(w, dir, coord, newTestAPIRespondRequest()) assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "response cannot be empty") }) @@ -4455,7 +4654,7 @@ func TestSPAMiddlewareRouting(t *testing.T) { handler := srv.spaMiddleware(mux) t.Run("apiRoutePassesThrough", func(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/api/v1/state", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/api/v1/workspaces", http.NoBody) w := httptest.NewRecorder() handler.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) @@ -4474,7 +4673,7 @@ func TestHandleSignalStream(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) req := httptest.NewRequest(http.MethodGet, "/api/v1/signal", http.NoBody).WithContext(ctx) - w := httptest.NewRecorder() + w := newLockedResponseRecorder() done := make(chan struct{}) go func() { @@ -4684,7 +4883,6 @@ func TestCheckWorkspaceState(t *testing.T) { srv.checkWorkspaceState(dir, snapshots, activeWorkspaces) assert.True(t, activeWorkspaces[dir]) assert.Contains(t, snapshots, dir) - assert.Equal(t, state.StatusComplete, snapshots[dir].status) } func TestCheckWorkspaceStateNoStateFile(t *testing.T) { @@ -5462,7 +5660,7 @@ func TestHandleAPIStateOmitsForkCommitFields(t *testing.T) { server.mu.Unlock() server.invalidateWorkspaceScanCache() - w := serveHTTP(server, "GET", "/api/v1/state", "") + w := serveHTTP(server, "GET", "/api/v1/workspaces", "") assert.Equal(t, http.StatusOK, w.Code) type forkState struct { @@ -5474,11 +5672,11 @@ func TestHandleAPIStateOmitsForkCommitFields(t *testing.T) { Name string `json:"name"` Forks []forkState `json:"forks"` } - type factoryState struct { + type workspaceListResponse struct { Workspaces []workspaceState `json:"workspaces"` } - var resp factoryState + var resp workspaceListResponse require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) var rootWorkspace *workspaceState @@ -5880,7 +6078,7 @@ func TestHandleAPIAdhocStopSuccess(t *testing.T) { func TestHandleAPIWorkflowSVGNotAvailable(t *testing.T) { server, rootDir := setupTestServer(t) setupTestWorkspace(t, server, rootDir, "ws-svg") - w := serveHTTP(server, "GET", "/api/v1/workspaces/ws-svg/workflow-svg", "") + w := serveHTTP(server, "GET", "/api/v1/workspaces/ws-svg/workflow.svg", "") assert.Equal(t, http.StatusNotFound, w.Code) } @@ -5890,27 +6088,277 @@ func TestSessionCoordinatorNoSession(t *testing.T) { } func TestHandleAPISignalStream(t *testing.T) { - server, _ := setupTestServer(t) - mux := http.NewServeMux() - server.registerAPIRoutes(mux) + synctest.Test(t, func(t *testing.T) { + server, _ := setupTestServer(t) + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + server.notifyStateChange() + synctest.Wait() + + assert.Contains(t, w.Header().Get("Content-Type"), "text/event-stream") + assert.Contains(t, w.BodyString(), "event: reload") + }) +} - ctx, cancel := context.WithCancel(context.Background()) - req := httptest.NewRequest(http.MethodGet, "/api/v1/signal", http.NoBody).WithContext(ctx) - w := httptest.NewRecorder() +func TestHandleAPISignalStreamDeliversReloadAndWorkspaceWhenNotificationsRace(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-coalesce") + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + server.notifyStateChange() + server.notifyWorkspaceListChange(wsDir) + synctest.Wait() + + assert.Contains(t, w.BodyString(), "event: reload") + assert.Contains(t, w.BodyString(), "event: workspace") + assert.Contains(t, w.BodyString(), filepath.Base(wsDir)) + }) +} - done := make(chan struct{}) - go func() { - mux.ServeHTTP(w, req) - close(done) - }() +func TestHandleAPISignalStreamUsesWorkspaceEventForLogUpdates(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-signal") - time.Sleep(50 * time.Millisecond) - server.notifyStateChange() - time.Sleep(50 * time.Millisecond) - cancel() - <-done + sess := newTestServeSession(nil, true) + sess.outputLog = newCircularLogBuffer() + server.mu.Lock() + server.sessions[wsDir] = sess + server.mu.Unlock() - assert.Contains(t, w.Header().Get("Content-Type"), "text/event-stream") + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + writer := newSessionLogWriter(sess, wsDir, server) + writer.addLine("line 1") + synctest.Wait() + + assert.Contains(t, w.BodyString(), "event: workspace") + assert.Contains(t, w.BodyString(), filepath.Base(wsDir)) + assert.NotContains(t, w.BodyString(), "event: reload") + }) +} + +func TestHandleAPISignalStreamUsesWorkspaceEventForProgressUpdates(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-progress") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Test"), 0o644)) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Progress = []state.ProgressEntry{{Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 1"}} + })) + + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + snapshots := make(map[string]workspaceStateSnapshot) + active := make(map[string]bool) + server.checkWorkspaceState(wsDir, snapshots, active) + server.loadWorkspaceListResponse() + + writeWorkflowStateWithLaterModTime(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Progress = []state.ProgressEntry{ + {Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 1"}, + {Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 2"}, + } + })) + + server.checkWorkspaceState(wsDir, snapshots, active) + synctest.Wait() + + assert.Contains(t, w.BodyString(), "event: workspace") + assert.Contains(t, w.BodyString(), filepath.Base(wsDir)) + assert.Contains(t, w.BodyString(), "event: reload") + }) +} + +func TestHandleAPISignalStreamUsesWorkspaceEventForStatusUpdates(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-status") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Test"), 0o644)) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + })) + + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + snapshots := make(map[string]workspaceStateSnapshot) + active := make(map[string]bool) + server.checkWorkspaceState(wsDir, snapshots, active) + server.loadWorkspaceListResponse() + + writeWorkflowStateWithLaterModTime(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusComplete + })) + + server.checkWorkspaceState(wsDir, snapshots, active) + synctest.Wait() + + assert.Contains(t, w.BodyString(), "event: workspace") + assert.Contains(t, w.BodyString(), filepath.Base(wsDir)) + assert.Contains(t, w.BodyString(), "event: reload") + }) +} + +func TestHandleAPISignalStreamUsesWorkspaceEventForWorkspacePageSummaryChanges(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-summary") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Test"), 0o644)) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "coordinator" + workflow.CurrentModel = "coordinator:gpt-4.1" + workflow.Task = "first task" + workflow.AgentSequence = []state.AgentSequenceEntry{{Agent: "coordinator", StartTime: "2026-03-30T00:00:00Z", IsCurrent: true}} + workflow.Cost = updated(newTestSessionCost(), func(cost *state.SessionCost) { + cost.TotalCost = 1.5 + }) + workflow.ModelStatuses = map[string]string{"coordinator:gpt-4.1": "model-working"} + })) + + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + snapshots := make(map[string]workspaceStateSnapshot) + active := make(map[string]bool) + server.checkWorkspaceState(wsDir, snapshots, active) + server.loadWorkspaceListResponse() + + writeWorkflowStateWithLaterModTime(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.CurrentModel = "go-developer:gpt-5.4" + workflow.Task = "second task" + workflow.AgentSequence = []state.AgentSequenceEntry{ + {Agent: "coordinator", StartTime: "2026-03-30T00:00:00Z", IsCurrent: false}, + {Agent: "go-developer", StartTime: "2026-03-30T00:01:00Z", IsCurrent: true}, + } + workflow.Cost = updated(newTestSessionCost(), func(cost *state.SessionCost) { + cost.TotalCost = 3.25 + }) + workflow.ModelStatuses = map[string]string{"go-developer:gpt-5.4": "model-done"} + })) + + server.checkWorkspaceState(wsDir, snapshots, active) + synctest.Wait() + + body := w.BodyString() + assert.Contains(t, body, "event: workspace") + assert.Contains(t, body, filepath.Base(wsDir)) + assert.Contains(t, body, "event: reload") + }) +} + +func TestHandleAPISignalStreamUsesReloadForSummaryChangesAfterWorkspaceListCacheExpiry(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-summary-cache-expiry") + attachRunningSessionCoordinator(t, server, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "before" + })) + + _ = server.loadWorkspaceListResponse() + _, errLoad := server.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + server.workspaceListCache.delete("workspaces") + + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "after" + }) + server.notifyWorkspaceChangeForState(wsDir, &wfState, true) + synctest.Wait() + + body := w.BodyString() + assert.Equal(t, 1, strings.Count(body, "event: reload")) + assert.Equal(t, 1, strings.Count(body, "event: workspace")) + assert.Contains(t, body, filepath.Base(wsDir)) + }) +} + +func TestHandleAPISignalStreamKeepsWorkspaceEventOnlyWhenWorkspaceListCacheExpiresAndSummaryMatches(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "ws-summary-cache-stable") + attachRunningSessionCoordinator(t, server, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + })) + + _ = server.loadWorkspaceListResponse() + _, errLoad := server.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + server.workspaceListCache.delete("workspaces") + + w, cancel, done := openSignalStream(t, server) + defer func() { + cancel() + synctest.Wait() + <-done + }() + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + workflow.Messages = []state.Message{messageWith(func(message *state.Message) { + message.ID = 1 + message.FromAgent = "go-developer" + message.ToAgent = "coordinator" + message.Body = "page-only change" + })} + }) + server.notifyWorkspaceChangeForState(wsDir, &wfState, true) + synctest.Wait() + + body := w.BodyString() + assert.Equal(t, 0, strings.Count(body, "event: reload")) + assert.Equal(t, 1, strings.Count(body, "event: workspace")) + assert.Contains(t, body, filepath.Base(wsDir)) + }) } func TestStartSessionAlreadyRunning(t *testing.T) { diff --git a/cmd/sgai/serve_test.go b/cmd/sgai/serve_test.go index c5ffeff..77fa728 100644 --- a/cmd/sgai/serve_test.go +++ b/cmd/sgai/serve_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "testing" + "testing/synctest" "time" "github.com/stretchr/testify/assert" @@ -46,6 +47,21 @@ func timestampProgressEntries(timestamps ...string) []state.ProgressEntry { return entries } +func assertWorkspaceChangeInvalidatesWorkspaceCaches(t *testing.T, srv *Server, wsDir string, wfState *state.Workflow, running bool) { + t.Helper() + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + srv.notifyWorkspaceChangeForState(wsDir, wfState, running) + + _, okList := srv.workspaceListCache.get("workspaces") + assert.False(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + func describedProgressEntries(descriptions ...string) []state.ProgressEntry { entries := make([]state.ProgressEntry, 0, len(descriptions)) for _, description := range descriptions { @@ -224,6 +240,7 @@ func TestSignalBrokerSubscribe(t *testing.T) { broker := newSignalBroker() sub := broker.subscribe() + defer broker.unsubscribe(sub) require.NotNil(t, sub) require.NotNil(t, sub.ch) require.NotNil(t, sub.done) @@ -257,35 +274,82 @@ func TestSignalBrokerNotify(t *testing.T) { sub1 := broker.subscribe() sub2 := broker.subscribe() + defer broker.unsubscribe(sub1) + defer broker.unsubscribe(sub2) - broker.notify() + broker.notify(signalEvent{Name: "workspace", Workspace: "ws-1"}) - select { - case <-sub1.ch: - default: - t.Fatal("subscriber 1 should receive notification") - } + var got1, got2 signalEvent + require.Eventually(t, func() bool { + select { + case got1 = <-sub1.ch: + default: + } + select { + case got2 = <-sub2.ch: + default: + } + return got1.Name != "" && got2.Name != "" + }, time.Second, 10*time.Millisecond) - select { - case <-sub2.ch: - default: - t.Fatal("subscriber 2 should receive notification") - } + assert.Equal(t, "workspace", got1.Name) + assert.Equal(t, "ws-1", got1.Workspace) + assert.Equal(t, "workspace", got2.Name) + assert.Equal(t, "ws-1", got2.Workspace) } func TestSignalBrokerNotifyWithFullChannel(t *testing.T) { broker := newSignalBroker() sub := broker.subscribe() + defer broker.unsubscribe(sub) - sub.ch <- struct{}{} - broker.notify() + preloaded := signalEvent{Name: "reload", Workspace: ""} + queued := signalEvent{Name: "workspace", Workspace: "ws-queued"} + sub.ch <- preloaded + broker.notify(queued) select { - case <-sub.ch: + case got := <-sub.ch: + assert.Equal(t, preloaded, got) default: - t.Fatal("should have one notification") + t.Fatal("should read the preloaded event first") } + + var got signalEvent + require.Eventually(t, func() bool { + select { + case got = <-sub.ch: + return got == queued + default: + return false + } + }, time.Second, 10*time.Millisecond) + + assert.Equal(t, queued, got) +} + +func TestSignalBrokerNotifyPreservesReloadAndWorkspaceSemantics(t *testing.T) { + broker := newSignalBroker() + sub := broker.subscribe() + defer broker.unsubscribe(sub) + + broker.notify(signalEvent{Name: "reload", Workspace: ""}) + broker.notify(signalEvent{Name: "workspace", Workspace: "ws-1"}) + + var events []signalEvent + require.Eventually(t, func() bool { + for { + select { + case event := <-sub.ch: + events = append(events, event) + default: + return len(events) == 2 + } + } + }, time.Second, 10*time.Millisecond) + + assert.Equal(t, []signalEvent{{Name: "reload", Workspace: ""}, {Name: "workspace", Workspace: "ws-1"}}, events) } func TestWorkspaceDagAgents(t *testing.T) { @@ -670,6 +734,36 @@ func TestResolveServerPaths(t *testing.T) { assert.Equal(t, want, paths.externalConfigDir) } +func TestParseServeOptions(t *testing.T) { + t.Run("usesWorkingDirectoryWhenRootIsOmitted", func(t *testing.T) { + cwd := t.TempDir() + configHome := t.TempDir() + + options, errParse := parseServeOptions([]string{"--listen-addr", "127.0.0.1:0"}, func() (string, error) { + return cwd, nil + }, configHome) + require.NoError(t, errParse) + assert.Equal(t, "127.0.0.1:0", options.listenAddr) + assert.Equal(t, cwd, options.rootDir) + assert.Equal(t, filepath.Join(configHome, "sgai"), options.paths.pinnedConfigDir) + assert.Equal(t, filepath.Join(configHome, "sgai"), options.paths.externalConfigDir) + }) + + t.Run("usesCustomSharedConfigDirectory", func(t *testing.T) { + cwd := t.TempDir() + sharedConfigDir := t.TempDir() + rootDir := t.TempDir() + + options, errParse := parseServeOptions([]string{"--shared-config-dir", sharedConfigDir, rootDir}, func() (string, error) { + return cwd, nil + }, t.TempDir()) + require.NoError(t, errParse) + assert.Equal(t, rootDir, options.rootDir) + assert.Equal(t, sharedConfigDir, options.paths.pinnedConfigDir) + assert.Equal(t, sharedConfigDir, options.paths.externalConfigDir) + }) +} + func TestAddGitExcludeNoGitDir(t *testing.T) { dir := t.TempDir() err := addGitExclude(dir) @@ -1183,17 +1277,228 @@ func TestClassifyWorkspaceCachedNonExistent(t *testing.T) { assert.Equal(t, workspaceStandalone, kind) } -func TestNotifyStateChangeInvalidatesCache(t *testing.T) { +func TestNotifyStateChangeInvalidatesWorkspaceCaches(t *testing.T) { srv, rootDir := setupTestServer(t) - _ = setupTestWorkspace(t, srv, rootDir, "notify-ws") + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-ws") - srv.warmStateCache() - _, ok := srv.stateCache.get("state") - assert.True(t, ok) + var listEntry apiWorkspaceListEntry + listEntry.Name = "notify-ws" + var pageState apiWorkspaceFullState + pageState.Name = "notify-ws" + + srv.workspaceListCache.set("workspaces", apiWorkspaceListResponse{Workspaces: []apiWorkspaceListEntry{listEntry}}) + srv.workspacePageCache.set(wsDir, pageState) srv.notifyStateChange() - _, ok2 := srv.stateCache.get("state") - assert.False(t, ok2) + _, okList := srv.workspaceListCache.get("workspaces") + assert.False(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + +func TestNotifyWorkspacePageChangeKeepsWorkspaceListCache(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-ws") + + var listEntry apiWorkspaceListEntry + listEntry.Name = "notify-workspace-ws" + var pageState apiWorkspaceFullState + pageState.Name = "notify-workspace-ws" + + srv.workspaceListCache.set("workspaces", apiWorkspaceListResponse{Workspaces: []apiWorkspaceListEntry{listEntry}}) + srv.workspacePageCache.set(wsDir, pageState) + + srv.notifyWorkspacePageChange(wsDir) + _, okList := srv.workspaceListCache.get("workspaces") + assert.True(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + +func TestNotifyWorkspacePageChangeSignalsStableWorkspacePathForDuplicateBasenames(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + srv, _ := setupTestServer(t) + firstDir := filepath.Join(t.TempDir(), "first", "shared-ws") + secondDir := filepath.Join(t.TempDir(), "second", "shared-ws") + require.NoError(t, os.MkdirAll(firstDir, 0o755)) + require.NoError(t, os.MkdirAll(secondDir, 0o755)) + + sub := srv.signals.subscribe() + defer func() { + srv.signals.unsubscribe(sub) + synctest.Wait() + }() + + eventsCh := make(chan []signalEvent, 1) + go func() { + events := make([]signalEvent, 0, 2) + for len(events) < 2 { + events = append(events, <-sub.ch) + } + eventsCh <- events + }() + synctest.Wait() + + srv.notifyWorkspacePageChange(firstDir) + srv.notifyWorkspacePageChange(secondDir) + synctest.Wait() + + assert.Equal(t, []signalEvent{ + {Name: "workspace", Workspace: workspaceSignalPath(firstDir)}, + {Name: "workspace", Workspace: workspaceSignalPath(secondDir)}, + }, <-eventsCh) + }) +} + +func TestNotifyWorkspaceListChangeInvalidatesWorkspaceListCache(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-list") + + var listEntry apiWorkspaceListEntry + listEntry.Name = "notify-workspace-list" + var pageState apiWorkspaceFullState + pageState.Name = "notify-workspace-list" + + srv.workspaceListCache.set("workspaces", apiWorkspaceListResponse{Workspaces: []apiWorkspaceListEntry{listEntry}}) + srv.workspacePageCache.set(wsDir, pageState) + + srv.notifyWorkspaceListChange(wsDir) + _, okList := srv.workspaceListCache.get("workspaces") + assert.False(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + +func TestNotifyWorkspaceChangeForStateKeepsWorkspaceListCacheWhenSummaryMatches(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-summary") + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + })) + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + workflow.Messages = []state.Message{messageWith(func(message *state.Message) { + message.ID = 1 + message.FromAgent = "go-developer" + message.ToAgent = "coordinator" + message.Body = "page-only change" + })} + }) + srv.notifyWorkspaceChangeForState(wsDir, &wfState, true) + + _, okList := srv.workspaceListCache.get("workspaces") + assert.True(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + +func TestNotifyWorkspaceChangeForStateKeepsWorkspaceListCacheWhenOnlyElapsedTimeDiffers(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-elapsed") + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + workflow.Progress = []state.ProgressEntry{progressEntryWith(func(entry *state.ProgressEntry) { + entry.Timestamp = time.Now().UTC().Format(time.RFC3339) + entry.Agent = "go-developer" + entry.Description = "still running" + })} + workflow.AgentSequence = []state.AgentSequenceEntry{agentSequenceEntryWith(func(entry *state.AgentSequenceEntry) { + entry.Agent = "go-developer" + entry.StartTime = time.Now().Add(-time.Minute).UTC().Format(time.RFC3339) + entry.IsCurrent = true + })} + })) + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + cached, okCache := srv.workspaceListCache.get("workspaces") + require.True(t, okCache) + require.Len(t, cached.Workspaces, 1) + cached.Workspaces[0].TotalExecTime = "stale elapsed time" + srv.workspaceListCache.set("workspaces", cached) + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "stable summary" + workflow.Progress = []state.ProgressEntry{progressEntryWith(func(entry *state.ProgressEntry) { + entry.Timestamp = time.Now().UTC().Format(time.RFC3339) + entry.Agent = "go-developer" + entry.Description = "still running" + })} + workflow.AgentSequence = []state.AgentSequenceEntry{agentSequenceEntryWith(func(entry *state.AgentSequenceEntry) { + entry.Agent = "go-developer" + entry.StartTime = time.Now().Add(-time.Minute).UTC().Format(time.RFC3339) + entry.IsCurrent = true + })} + workflow.Messages = []state.Message{messageWith(func(message *state.Message) { + message.ID = 1 + message.FromAgent = "go-developer" + message.ToAgent = "coordinator" + message.Body = "page-only change" + })} + }) + srv.notifyWorkspaceChangeForState(wsDir, &wfState, true) + + _, okList := srv.workspaceListCache.get("workspaces") + assert.True(t, okList) + _, okPage := srv.workspacePageCache.get(wsDir) + assert.False(t, okPage) +} + +func TestNotifyWorkspaceChangeForStateInvalidatesWorkspaceListCacheWhenRunningChanges(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-running-change") + attachSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "resume work" + })) + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "resume work" + }) + assertWorkspaceChangeInvalidatesWorkspaceCaches(t, srv, wsDir, &wfState, true) +} + +func TestNotifyWorkspaceChangeForStateInvalidatesWorkspaceListCacheWhenSummaryChanges(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "notify-workspace-summary-change") + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "before" + })) + + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + wfState := workflowWith(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.CurrentAgent = "go-developer" + workflow.Task = "after" + }) + assertWorkspaceChangeInvalidatesWorkspaceCaches(t, srv, wsDir, &wfState, true) } func TestWorkspaceCoordinator(t *testing.T) { diff --git a/cmd/sgai/service_adhoc.go b/cmd/sgai/service_adhoc.go index 2415a2f..b9b7216 100644 --- a/cmd/sgai/service_adhoc.go +++ b/cmd/sgai/service_adhoc.go @@ -223,10 +223,10 @@ func (s *Server) startCommandService(workspacePath string, spec *commandStartSpe _, _ = fmt.Fprintln(stderrPW, "command exited with error:", errWait) _, _ = fmt.Fprintln(stderrBufferPW, "command exited with error:", errWait) } - s.notifyStateChange() + s.notifyWorkspacePageChange(workspacePath) }() - s.notifyStateChange() + s.notifyWorkspacePageChange(workspacePath) return adhocStartRunning("", spec.startedMessage) } @@ -240,7 +240,7 @@ type adhocStopResult struct { func (s *Server) adhocStopService(workspacePath string) adhocStopResult { st := s.getAdhocState(workspacePath) st.stop() - s.notifyStateChange() + s.notifyWorkspacePageChange(workspacePath) st.mu.Lock() output := st.output.String() diff --git a/cmd/sgai/service_session.go b/cmd/sgai/service_session.go index 8025e73..a0803a5 100644 --- a/cmd/sgai/service_session.go +++ b/cmd/sgai/service_session.go @@ -16,6 +16,10 @@ 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") + errResponseCannotBeEmpty = errors.New("response cannot be empty") + errQuestionNotAvailable = errors.New("question not available") + errSteerMessageEmpty = errors.New("message cannot be empty") ) type sessionStartResult struct { @@ -82,8 +86,6 @@ func (s *Server) startSessionService(workspacePath string, auto bool) (sessionSt return sessionStartResult{}, result.startError } - s.notifyStateChange() - return sessionStartResult{ Name: name, Status: "running", @@ -179,8 +181,6 @@ func (s *Server) stopSessionService(workspacePath string) stopSessionResult { message = "session already stopped" } - s.notifyStateChange() - return stopSessionResult{ Name: filepath.Base(workspacePath), Status: "stopped", @@ -217,7 +217,7 @@ func (s *Server) resetSessionService(workspacePath string) (resetSessionResult, return resetSessionResult{}, fmt.Errorf("failed to reset state: %w", errUpdate) } - s.notifyStateChange() + s.notifyWorkspaceChangeAfterCoordinatorUpdate(workspacePath, coord) return resetSessionResult{ Name: filepath.Base(workspacePath), @@ -243,28 +243,28 @@ func (s *Server) respondService(workspacePath, promptToken, answer string, selec coord := s.sessionCoordinator(workspacePath) if coord != nil { log.Println("respond-service:", wsName, "delivering via session coordinator") - return s.respondViaCoordinatorService(coord, req) + return s.respondViaCoordinatorService(workspacePath, coord, req) } log.Println("respond-service:", wsName, "rejected, no session coordinator found") - return respondResult{}, errors.New("no pending question") + return respondResult{}, errNoPendingQuestion } -func (s *Server) respondViaCoordinatorService(coord *state.Coordinator, req apiRespondRequest) (respondResult, error) { +func (s *Server) respondViaCoordinatorService(workspacePath string, coord *state.Coordinator, req apiRespondRequest) (respondResult, error) { wfState := coord.State() if !wfState.NeedsHumanInput() { log.Println("respond-service: coordinator path rejected, no pending question, status:", wfState.Status) - return respondResult{}, errors.New("no pending question") + return respondResult{}, errNoPendingQuestion } responseText := buildAPIResponseText(req) if responseText == "" { - return respondResult{}, errors.New("response cannot be empty") + return respondResult{}, errResponseCannotBeEmpty } if !coord.RespondIfCurrent(req.PromptToken, responseText) { - return respondResult{}, errors.New("question not available") + return respondResult{}, errQuestionNotAvailable } if wfState.MultiChoiceQuestion != nil && wfState.MultiChoiceQuestion.IsWorkGate { @@ -279,7 +279,7 @@ func (s *Server) respondViaCoordinatorService(coord *state.Coordinator, req apiR } } } - s.notifyStateChange() + s.notifyWorkspaceChangeAfterCoordinatorUpdate(workspacePath, coord) return respondResult{Success: true, Message: "response submitted"}, nil } @@ -291,7 +291,7 @@ type steerResult struct { func (s *Server) steerService(workspacePath, message string) (steerResult, error) { if strings.TrimSpace(message) == "" { - return steerResult{}, errors.New("message cannot be empty") + return steerResult{}, errSteerMessageEmpty } coord := s.workspaceCoordinator(workspacePath) @@ -315,7 +315,7 @@ func (s *Server) steerService(workspacePath, message string) (steerResult, error return steerResult{}, fmt.Errorf("failed to save state: %w", errUpdate) } - s.notifyStateChange() + s.notifyWorkspaceChangeAfterCoordinatorUpdate(workspacePath, coord) return steerResult{Success: true, Message: "steering instruction added"}, nil } diff --git a/cmd/sgai/service_session_test.go b/cmd/sgai/service_session_test.go index 5608165..493da00 100644 --- a/cmd/sgai/service_session_test.go +++ b/cmd/sgai/service_session_test.go @@ -3,36 +3,73 @@ package main import ( "context" "net/http" + "net/http/httptest" "os" "path/filepath" + "strings" "testing" - "time" + "testing/synctest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/ucirello/sgai/pkg/state" ) +func openSignalStream(t *testing.T, server *Server) (*lockedResponseRecorder, context.CancelFunc, <-chan struct{}) { + t.Helper() + mux := http.NewServeMux() + server.registerAPIRoutes(mux) + + ctx, cancel := context.WithCancel(context.Background()) + req := httptest.NewRequest(http.MethodGet, "/api/v1/signal", http.NoBody).WithContext(ctx) + w := newLockedResponseRecorder() + + done := make(chan struct{}) + go func() { + mux.ServeHTTP(w, req) + close(done) + }() + + synctest.Wait() + server.signals.mu.Lock() + defer server.signals.mu.Unlock() + require.Len(t, server.signals.subscribers, 1) + + return w, cancel, done +} + +func completeStartedSessionForTest(t *testing.T, server *Server, workspacePath string, sess *session, coord *state.Coordinator) { + t.Helper() + server.finishSessionRun(workspacePath, sess, coord) +} + func startCoordinatorQuestion(t *testing.T, coord *state.Coordinator, question *state.MultiChoiceQuestion, humanMessage string) (<-chan error, context.CancelFunc) { t.Helper() + ready := make(chan struct{}, 1) + coord.OnUpdate(func() { + if coord.State().NeedsHumanInput() { + select { + case ready <- struct{}{}: + default: + } + } + }) ctx, cancel := context.WithCancel(context.Background()) errCh := make(chan error, 1) go func() { _, err := coord.AskAndWait(ctx, question, humanMessage) errCh <- err }() - require.Eventually(t, func() bool { - return coord.State().NeedsHumanInput() - }, time.Second, 10*time.Millisecond) + <-ready + require.True(t, coord.State().NeedsHumanInput()) return errCh, cancel } func waitForSessionPromptToken(t *testing.T, coord *state.Coordinator) string { t.Helper() - require.Eventually(t, func() bool { - return coord.CurrentPromptToken() != "" - }, time.Second, 10*time.Millisecond) - return coord.CurrentPromptToken() + promptToken := coord.CurrentPromptToken() + require.NotEmpty(t, promptToken) + return promptToken } func TestStopSessionService(t *testing.T) { @@ -94,6 +131,7 @@ func TestRespondService(t *testing.T) { selectedChoices []string setupFunc func(*testing.T, string) wantErr bool + expectedErr error errContains string validate func(*testing.T, respondResult) }{ @@ -107,6 +145,7 @@ func TestRespondService(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errNoPendingQuestion, errContains: "no pending question", validate: nil, }, @@ -120,6 +159,7 @@ func TestRespondService(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errNoPendingQuestion, errContains: "no pending question", validate: nil, }, @@ -138,6 +178,9 @@ func TestRespondService(t *testing.T) { if tt.wantErr { require.Error(t, err) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) + } if tt.errContains != "" { assert.Contains(t, err.Error(), tt.errContains) } @@ -158,6 +201,7 @@ func TestSteerService(t *testing.T) { message string setupFunc func(*testing.T, string) wantErr bool + expectedErr error errContains string validate func(*testing.T, steerResult) }{ @@ -169,6 +213,7 @@ func TestSteerService(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: false, + expectedErr: nil, errContains: "", validate: func(t *testing.T, result steerResult) { t.Helper() @@ -184,6 +229,7 @@ func TestSteerService(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errSteerMessageEmpty, errContains: "message cannot be empty", validate: nil, }, @@ -195,6 +241,7 @@ func TestSteerService(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errSteerMessageEmpty, errContains: "message cannot be empty", validate: nil, }, @@ -213,6 +260,9 @@ func TestSteerService(t *testing.T) { if tt.wantErr { require.Error(t, err) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) + } if tt.errContains != "" { assert.Contains(t, err.Error(), tt.errContains) } @@ -254,6 +304,7 @@ func TestRespondServiceValidation(t *testing.T) { selectedChoices []string setupFunc func(*testing.T, string) wantErr bool + expectedErr error errContains string }{ { @@ -266,6 +317,7 @@ func TestRespondServiceValidation(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errNoPendingQuestion, errContains: "no pending question", }, { @@ -278,6 +330,7 @@ func TestRespondServiceValidation(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errNoPendingQuestion, errContains: "no pending question", }, { @@ -290,6 +343,7 @@ func TestRespondServiceValidation(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) }, wantErr: true, + expectedErr: errNoPendingQuestion, errContains: "no pending question", }, } @@ -307,6 +361,9 @@ func TestRespondServiceValidation(t *testing.T) { if tt.wantErr { require.Error(t, err) + if tt.expectedErr != nil { + require.ErrorIs(t, err, tt.expectedErr) + } if tt.errContains != "" { assert.Contains(t, err.Error(), tt.errContains) } @@ -333,8 +390,9 @@ func TestRespondViaCoordinatorServiceNoQuestion(t *testing.T) { request.Answer = "Test answer" }) - _, err := server.respondViaCoordinatorService(coord, req) + _, err := server.respondViaCoordinatorService(workspacePath, coord, req) require.Error(t, err) + require.ErrorIs(t, err, errNoPendingQuestion) assert.Contains(t, err.Error(), "no pending question") } @@ -355,87 +413,96 @@ func TestRespondViaCoordinatorServiceEmptyResponse(t *testing.T) { request.Answer = "" }) - _, err := server.respondViaCoordinatorService(coord, req) + _, err := server.respondViaCoordinatorService(workspacePath, coord, req) require.Error(t, err) + require.ErrorIs(t, err, errResponseCannotBeEmpty) assert.Contains(t, err.Error(), "response cannot be empty") } func TestRespondViaCoordinatorServiceWorkGateApproval(t *testing.T) { - rootDir := t.TempDir() - server := NewServer(rootDir, newTestServerPaths(), "") + synctest.Test(t, func(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + + workspacePath := filepath.Join(rootDir, "test-workspace") + require.NoError(t, os.MkdirAll(workspacePath, 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) + + coord := state.NewCoordinatorEmpty(statePath(workspacePath)) + require.NoError(t, coord.UpdateState(func(wf *state.Workflow) { + wf.InteractionMode = state.ModeBrainstorming + })) + errCh, cancel := startCoordinatorQuestion(t, coord, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Approve this definition?" + item.Choices = []string{workGateApprovalText, "Not ready"} + })} + question.IsWorkGate = true + }), "Approve this definition?") + defer cancel() - workspacePath := filepath.Join(rootDir, "test-workspace") - require.NoError(t, os.MkdirAll(workspacePath, 0o755)) - require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) + req := respondRequestWith(func(request *apiRespondRequest) { + request.SelectedChoices = []string{workGateApprovalText} + }) - coord := state.NewCoordinatorEmpty(statePath(workspacePath)) - require.NoError(t, coord.UpdateState(func(wf *state.Workflow) { - wf.InteractionMode = state.ModeBrainstorming - })) - errCh, cancel := startCoordinatorQuestion(t, coord, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { - question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { - item.Question = "Approve this definition?" - item.Choices = []string{workGateApprovalText, "Not ready"} - })} - question.IsWorkGate = true - }), "Approve this definition?") - defer cancel() + result, err := server.respondViaCoordinatorService(workspacePath, coord, req) + require.NoError(t, err) + assert.True(t, result.Success) + synctest.Wait() + require.NoError(t, <-errCh) - req := respondRequestWith(func(request *apiRespondRequest) { - request.SelectedChoices = []string{workGateApprovalText} + wfState := coord.State() + assert.Equal(t, state.ModeBuilding, wfState.InteractionMode) }) - - result, err := server.respondViaCoordinatorService(coord, req) - require.NoError(t, err) - assert.True(t, result.Success) - require.NoError(t, <-errCh) - - wfState := coord.State() - assert.Equal(t, state.ModeBuilding, wfState.InteractionMode) } func TestRespondViaCoordinatorServiceRejectsStalePromptToken(t *testing.T) { - rootDir := t.TempDir() - server := NewServer(rootDir, newTestServerPaths(), "") - workspacePath := filepath.Join(rootDir, "test-workspace") - require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) - - coord := state.NewCoordinatorEmpty(statePath(workspacePath)) - firstErrCh, cancelFirst := startCoordinatorQuestion(t, coord, 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") - firstToken := waitForSessionPromptToken(t, coord) - cancelFirst() - require.ErrorIs(t, <-firstErrCh, context.Canceled) - - secondErrCh, cancelSecond := startCoordinatorQuestion(t, coord, 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 cancelSecond() - secondToken := waitForSessionPromptToken(t, coord) - require.NotEqual(t, firstToken, secondToken) - - _, err := server.respondViaCoordinatorService(coord, respondRequestWith(func(request *apiRespondRequest) { - request.PromptToken = firstToken - request.Answer = "stale answer" - })) - require.Error(t, err) - assert.Contains(t, err.Error(), "question not available") - assert.True(t, coord.State().NeedsHumanInput()) + synctest.Test(t, func(t *testing.T) { + rootDir := t.TempDir() + server := NewServer(rootDir, newTestServerPaths(), "") + workspacePath := filepath.Join(rootDir, "test-workspace") + require.NoError(t, os.MkdirAll(filepath.Join(workspacePath, ".sgai"), 0o755)) + + coord := state.NewCoordinatorEmpty(statePath(workspacePath)) + firstErrCh, cancelFirst := startCoordinatorQuestion(t, coord, 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") + firstToken := waitForSessionPromptToken(t, coord) + cancelFirst() + synctest.Wait() + require.ErrorIs(t, <-firstErrCh, context.Canceled) - result, err := server.respondViaCoordinatorService(coord, respondRequestWith(func(request *apiRespondRequest) { - request.PromptToken = secondToken - request.Answer = "current answer" - })) - require.NoError(t, err) - assert.True(t, result.Success) - require.NoError(t, <-secondErrCh) + secondErrCh, cancelSecond := startCoordinatorQuestion(t, coord, 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 cancelSecond() + secondToken := waitForSessionPromptToken(t, coord) + require.NotEqual(t, firstToken, secondToken) + + _, err := server.respondViaCoordinatorService(workspacePath, coord, respondRequestWith(func(request *apiRespondRequest) { + request.PromptToken = firstToken + request.Answer = "stale answer" + })) + require.Error(t, err) + require.ErrorIs(t, err, errQuestionNotAvailable) + assert.Contains(t, err.Error(), "question not available") + assert.True(t, coord.State().NeedsHumanInput()) + + result, err := server.respondViaCoordinatorService(workspacePath, coord, respondRequestWith(func(request *apiRespondRequest) { + request.PromptToken = secondToken + request.Answer = "current answer" + })) + require.NoError(t, err) + assert.True(t, result.Success) + synctest.Wait() + require.NoError(t, <-secondErrCh) + }) } func TestHandleRespondViaCoordinatorNoQuestion(t *testing.T) { @@ -485,35 +552,38 @@ func TestRespondViaCoordinatorEmptyResponse(t *testing.T) { } func TestRespondViaCoordinatorWorkGateApproval(t *testing.T) { - srv, rootDir := setupTestServer(t) - wsDir := setupTestWorkspace(t, srv, rootDir, "respond-gate") - require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\n---\n# Goal"), 0o644)) - - statePath := filepath.Join(wsDir, ".sgai", "state.json") - coord, errCoord := state.NewCoordinatorWith(statePath, workflowWith(func(workflow *state.Workflow) { - workflow.InteractionMode = state.ModeBrainstorming - })) - require.NoError(t, errCoord) - errCh, cancel := startCoordinatorQuestion(t, coord, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { - question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { - item.Question = "Is ready?" - item.Choices = []string{workGateApprovalText, "Not ready"} - })} - question.IsWorkGate = true - }), "Is this ready?") - defer cancel() - - srv.mu.Lock() - srv.sessions[wsDir] = newTestServeSession(coord, false) - srv.mu.Unlock() - - body := `{"answer":"","selectedChoices":["` + workGateApprovalText + `"]}` - w := serveHTTP(srv, "POST", "/api/v1/workspaces/respond-gate/respond", body) - assert.Equal(t, http.StatusOK, w.Code) - require.NoError(t, <-errCh) - - updatedState := coord.State() - assert.Equal(t, state.ModeBuilding, updatedState.InteractionMode) + synctest.Test(t, func(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "respond-gate") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("---\n---\n# Goal"), 0o644)) + + statePath := filepath.Join(wsDir, ".sgai", "state.json") + coord, errCoord := state.NewCoordinatorWith(statePath, workflowWith(func(workflow *state.Workflow) { + workflow.InteractionMode = state.ModeBrainstorming + })) + require.NoError(t, errCoord) + errCh, cancel := startCoordinatorQuestion(t, coord, multiChoiceQuestionWith(func(question *state.MultiChoiceQuestion) { + question.Questions = []state.QuestionItem{questionItemWith(func(item *state.QuestionItem) { + item.Question = "Is ready?" + item.Choices = []string{workGateApprovalText, "Not ready"} + })} + question.IsWorkGate = true + }), "Is this ready?") + defer cancel() + + srv.mu.Lock() + srv.sessions[wsDir] = newTestServeSession(coord, false) + srv.mu.Unlock() + + body := `{"answer":"","selectedChoices":["` + workGateApprovalText + `"]}` + w := serveHTTP(srv, "POST", "/api/v1/workspaces/respond-gate/respond", body) + assert.Equal(t, http.StatusOK, w.Code) + synctest.Wait() + require.NoError(t, <-errCh) + + updatedState := coord.State() + assert.Equal(t, state.ModeBuilding, updatedState.InteractionMode) + }) } func TestStopSessionServiceRunningSession(t *testing.T) { @@ -527,6 +597,62 @@ func TestStopSessionServiceRunningSession(t *testing.T) { assert.False(t, result.Running) } +func TestStopSessionPublishesSingleReloadAndWorkspaceSignals(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + server, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, server, rootDir, "stop-signal-ws") + + coord := state.NewCoordinatorEmpty(statePath(wsDir)) + coord.OnUpdate(func() { + wfState := coord.State() + server.notifyWorkspaceChangeForState(wsDir, &wfState, server.sessionRunning(wsDir)) + }) + + ctx, cancelWait := context.WithCancel(context.Background()) + sess := newTestServeSession(coord, true) + sess.cancel = cancelWait + sess.mcpCloseFn = func() {} + + server.mu.Lock() + server.sessions[wsDir] = sess + server.mu.Unlock() + + errCh := make(chan error, 1) + go func() { + _, err := coord.AskAndWait(ctx, nil, "question?") + errCh <- err + }() + + synctest.Wait() + require.True(t, coord.State().NeedsHumanInput()) + + _ = server.loadWorkspaceListResponse() + _, errLoad := server.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) + + w, cancelStream, done := openSignalStream(t, server) + server.stopSession(wsDir) + synctest.Wait() + require.ErrorIs(t, <-errCh, context.Canceled) + + completeStartedSessionForTest(t, server, wsDir, sess, coord) + synctest.Wait() + + cancelStream() + synctest.Wait() + <-done + + body := w.BodyString() + assert.Equal(t, 1, strings.Count(body, "event: reload")) + assert.Equal(t, 1, strings.Count(body, "event: workspace")) + assert.Contains(t, body, filepath.Base(wsDir)) + + wfState := coord.State() + assert.Empty(t, wfState.HumanMessage) + assert.Nil(t, wfState.MultiChoiceQuestion) + }) +} + func TestRespondServiceNoSession(t *testing.T) { server, rootDir := setupTestServer(t) wsDir := setupTestWorkspace(t, server, rootDir, "test-ws") @@ -537,6 +663,7 @@ func TestRespondServiceNoSession(t *testing.T) { require.NoError(t, errCoord) _, errRespond := server.respondService(wsDir, "wrong-id", "answer", nil) require.Error(t, errRespond) + require.ErrorIs(t, errRespond, errNoPendingQuestion) } func TestSteerServiceSuccessful(t *testing.T) { @@ -555,6 +682,7 @@ func TestSteerServiceEmptyMessageFails(t *testing.T) { wsDir := setupTestWorkspace(t, server, rootDir, "test-ws") _, errSteer := server.steerService(wsDir, " ") require.Error(t, errSteer) + require.ErrorIs(t, errSteer, errSteerMessageEmpty) } func TestStopSessionServiceNotRunning(t *testing.T) { diff --git a/cmd/sgai/service_workspace.go b/cmd/sgai/service_workspace.go index c5504f5..91978ba 100644 --- a/cmd/sgai/service_workspace.go +++ b/cmd/sgai/service_workspace.go @@ -224,7 +224,7 @@ func (s *Server) updateGoalService(workspacePath, content string) (updateGoalRes s.svgCache.deleteFunc(func(k string) bool { return strings.HasPrefix(k, prefix) }) - s.notifyStateChange() + s.notifyWorkspaceListChange(workspacePath) return updateGoalResult{Updated: true, Workspace: filepath.Base(workspacePath)}, nil } @@ -239,8 +239,7 @@ func (s *Server) togglePinService(workspacePath string) (togglePinResult, error) return togglePinResult{}, fmt.Errorf("failed to toggle pin: %w", errToggle) } - s.invalidateWorkspaceScanCache() - s.notifyStateChange() + s.notifyWorkspaceListChange(workspacePath) return togglePinResult{Pinned: s.isPinned(workspacePath), Message: "pin toggled"}, nil } @@ -286,7 +285,7 @@ func (s *Server) deleteMessageService(workspacePath string, messageID int) (dele return deleteMessageResult{}, fmt.Errorf("failed to save workspace state: %w", errUpdate) } - s.notifyStateChange() + s.notifyWorkspaceChangeAfterCoordinatorUpdate(workspacePath, coord) return deleteMessageResult{Deleted: true, ID: messageID, Message: "message deleted successfully"}, nil } diff --git a/cmd/sgai/skel/.sgai/agent/general-purpose.md b/cmd/sgai/skel/.sgai/agent/general-purpose.md index ea0b91c..a2ccadf 100644 --- a/cmd/sgai/skel/.sgai/agent/general-purpose.md +++ b/cmd/sgai/skel/.sgai/agent/general-purpose.md @@ -141,6 +141,13 @@ You can communicate with other agents using the messaging system: - When specialist review judgment is needed, route to the appropriate reviewer agent or ask the coordinator to do so. - Keep your own status messages grounded in implementation evidence rather than reviewer authority. +### Overlay path discipline for factory files + +- When a task involves prompts, skills, snippets, or other factory files currently visible under `.sgai/...`, treat those as read-only runtime copies. +- Read from `.sgai/...` when you need the live content, but write all durable changes to the matching `sgai/...` overlay path instead. +- Never leave prompt/skill/process fixes only under `.sgai/...`; those runtime files are regenerated and the change will be lost. +- In status updates and handoff notes, name the final `sgai/...` path you changed so the coordinator can verify the durable overlay location quickly. + --- ## Your Process diff --git a/cmd/sgai/skel/.sgai/agent/htmx-picocss-frontend-developer.md b/cmd/sgai/skel/.sgai/agent/htmx-picocss-frontend-developer.md index 085f2e0..026ae82 100644 --- a/cmd/sgai/skel/.sgai/agent/htmx-picocss-frontend-developer.md +++ b/cmd/sgai/skel/.sgai/agent/htmx-picocss-frontend-developer.md @@ -415,11 +415,11 @@ You MUST use Playwright screenshots to verify your work. This is not optional. ### Screenshot Storage -All screenshots must be stored in the retrospective's screenshots directory: +All Playwright evidence examples must use the retrospective session path declared in `.sgai/PROJECT_MANAGEMENT.md` frontmatter and store screenshots under: ``` -.sgai/retrospectives/screenshots// +.sgai/retrospectives//screenshots/ ``` -(the full path for the current session retrospective directory can be found in .sgai/PROJECT_MANAGEMENT.md frontmatter) +For example, if frontmatter declares `Retrospective Session: .sgai/retrospectives/2026-03-30-07-22.2io5`, the screenshot directory is `.sgai/retrospectives/2026-03-30-07-22.2io5/screenshots/`. ### Color Contrast Verification @@ -443,7 +443,7 @@ await playwright_browser_wait_for({time: 2}); // Take screenshot for visual verification await playwright_browser_take_screenshot({ - filename: ".sgai/retrospectives/screenshots//color-contrast-check.png" + filename: ".sgai/retrospectives//screenshots/color-contrast-check.png" }); // Examine the screenshot to verify: @@ -466,4 +466,3 @@ Build beautiful, fast, and accessible web interfaces using HTMX and PicoCSS. Foc - ALWAYS preserve scroll position on auto-refresh - ALWAYS use Playwright screenshots to verify your work - ALWAYS verify color contrasts are accessible - diff --git a/cmd/sgai/skel/.sgai/agent/react-developer.md b/cmd/sgai/skel/.sgai/agent/react-developer.md index b2897da..a640de2 100644 --- a/cmd/sgai/skel/.sgai/agent/react-developer.md +++ b/cmd/sgai/skel/.sgai/agent/react-developer.md @@ -435,11 +435,11 @@ You MUST use Playwright screenshots to verify your work. This is not optional. ### Screenshot Storage -All screenshots must be stored in the retrospective's screenshots directory: +All Playwright evidence examples must use the retrospective session path declared in `.sgai/PROJECT_MANAGEMENT.md` frontmatter and store screenshots under: ``` -.sgai/retrospectives/screenshots// +.sgai/retrospectives//screenshots/ ``` -(the full path for the current session retrospective directory can be found in .sgai/PROJECT_MANAGEMENT.md frontmatter) +For example, if frontmatter declares `Retrospective Session: .sgai/retrospectives/2026-03-30-07-22.2io5`, the screenshot directory is `.sgai/retrospectives/2026-03-30-07-22.2io5/screenshots/`. ### Verification Workflow @@ -450,7 +450,7 @@ await playwright_browser_wait_for({time: 2}); // Take screenshot for visual verification await playwright_browser_take_screenshot({ - filename: ".sgai/retrospectives/screenshots//react-component-check.png" + filename: ".sgai/retrospectives//screenshots/react-component-check.png" }); // Verify interactive behavior diff --git a/cmd/sgai/skel/.sgai/skills/browser-bug-testing-workflow/SKILL.md b/cmd/sgai/skel/.sgai/skills/browser-bug-testing-workflow/SKILL.md index 3e2eb20..77c8bdb 100644 --- a/cmd/sgai/skel/.sgai/skills/browser-bug-testing-workflow/SKILL.md +++ b/cmd/sgai/skel/.sgai/skills/browser-bug-testing-workflow/SKILL.md @@ -1,6 +1,6 @@ --- name: browser-bug-testing-workflow -description: Use when debugging a web UI bug or validating a multi-step browser flow. Start or reuse background servers through run-long-running-processes-in-tmux using the deterministic current-directory-plus-purpose session name, not generic tmux names or kill-before-create setup. +description: Use when debugging a web UI bug or validating a multi-step browser flow. Start or reuse background servers through run-long-running-processes-in-tmux using the deterministic current-directory-plus-purpose session name, not generic tmux names or kill-before-create setup. Prevent repo-root Playwright artifact leaks by resolving the retrospective screenshots directory once and prefixing every saved filename with it. metadata: requires: - run-long-running-processes-in-tmux @@ -12,6 +12,8 @@ metadata: Turn a browser bug into a repeatable loop: start or reuse the correct background server, drive the UI with Playwright, capture visual evidence, and preserve reusable tmux sessions across iterations. +Core principle: resolve the retrospective `screenshots/` directory before the first Playwright save, then prefix every saved filename with that exact repo-relative directory. Bare filenames are evidence leaks. + ## When to Use - Debugging interactive browser bugs @@ -39,7 +41,11 @@ Before the first Playwright save, determine the authoritative output directory a - Read `.sgai/PROJECT_MANAGEMENT.md`. - If its frontmatter declares `Retrospective Session: .sgai/retrospectives/`, treat `/screenshots/` as the required browser-evidence directory for the current session. - Create or verify that exact `screenshots/` directory before the first screenshot, snapshot, console log, or network log save. +- Resolve one repo-relative `evidence_dir` value such as `.sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots` and reuse it for **every** Playwright `filename` parameter in the run. +- Prefix every saved filename with that exact directory: screenshots, snapshots, console logs, network logs, and any other browser artifact saves. - Use that declared retrospective path exactly; do **not** improvise alternate layouts like `.sgai/retrospectives/screenshots//`. +- Do **not** pass bare filenames like `result.png`, `state.md`, or `console.txt`; from repo root they land in the repo root and pollute the workspace. +- The Playwright hint to prefer relative filenames means repo-relative paths under `evidence_dir`, **not** basenames with no directory. - If no retrospective-session frontmatter exists, still preflight the intended output directory before the first Playwright save instead of assuming the path already exists. ## Workflow @@ -64,11 +70,13 @@ fi - [ ] Read `.sgai/PROJECT_MANAGEMENT.md` for the current session path when it exists. - [ ] Resolve the required browser-evidence directory. - [ ] Create or verify that directory before the first Playwright save. +- [ ] Build exact repo-relative filenames under that directory before the first Playwright save. ### Step 3: Capture a baseline in the browser - [ ] Navigate to the target page. - [ ] Take an accessibility snapshot. - [ ] Take a screenshot when visual layout or styling matters. +- [ ] Save both with filenames prefixed by the resolved browser-evidence directory. ### Step 4: Perform the interaction - [ ] Click, type, select, or submit using exact element refs from snapshots. @@ -79,6 +87,7 @@ fi - [ ] Capture another snapshot. - [ ] Take a screenshot of the rendered result. - [ ] Capture console or network output when it helps explain the bug. +- [ ] Save every resulting artifact with filenames prefixed by the resolved browser-evidence directory. ### Step 6: Preserve evidence and clean up deliberately - [ ] Capture tmux pane output when server logs matter. @@ -92,6 +101,7 @@ fi 3. **Reuse keeps feedback loops fast** - Do not restart a healthy reusable server by default. 4. **Cleanup must be deliberate** - No unconditional `tmux kill-session` at the end of every browser pass. 5. **Evidence paths must be preflighted** - If `.sgai/PROJECT_MANAGEMENT.md` declares a retrospective session, use that session's `screenshots/` directory and ensure it exists before saving artifacts. +6. **Bare filenames are forbidden** - Every Playwright `filename` must include the resolved repo-relative browser-evidence directory prefix. ## Rationalization Table @@ -101,6 +111,8 @@ fi | "I'll restart to be safe before every browser run." | Reuse first, inspect logs, and restart only when the session is missing or broken. | | "This is a one-off browser test, so naming does not matter." | Stable naming is what makes the next iteration reusable instead of disposable. | | "Playwright will create the parent directory for me." | Parent directories often do not exist in session-specific evidence paths; preflight them before the first save. | +| "The tool says to prefer relative filenames, so `shot.png` is fine." | A basename is repo-root relative here; the safe relative filename is `.sgai/retrospectives//screenshots/shot.png`. | +| "I already used the right directory for snapshots/logs, so one bare screenshot name is harmless." | One bare filename is enough to pollute the repo root and fail the hygiene goal. | ## Red Flags - STOP @@ -110,6 +122,9 @@ fi - Repeated sleeps instead of condition-based waits and visual checks - Saving Playwright artifacts into an unverified path - Ignoring the `Retrospective Session:` frontmatter path in `.sgai/PROJECT_MANAGEMENT.md` +- `playwright_browser_take_screenshot(filename: 'result.png', ...)` +- `playwright_browser_snapshot(filename: 'state.md', ...)` +- `playwright_browser_console_messages(filename: 'console.txt', ...)` ## Examples @@ -117,27 +132,33 @@ fi ```bash session_name="Users-username-src-github-com-repo-slim-gray-i2e3--web" +evidence_dir=".sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots" if ! tmux has-session -t "$session_name" 2>/dev/null; then tmux new-session -d -s "$session_name" "bun run dev" fi tmux capture-pane -t "$session_name" -S - -p -mkdir -p ".sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots" +mkdir -p "$evidence_dir" ``` -Then use Playwright to navigate, interact, snapshot, and screenshot the result into the preflighted output directory. +Then save Playwright artifacts as paths like: + +- `.sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots/baseline.md` +- `.sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots/baseline.png` +- `.sgai/retrospectives/2026-03-29-09-20.ngyj/screenshots/console.txt` ### Bad Example ```bash tmux new-session -d -s webserver "npm run dev" sleep 3 -playwright save ".sgai/retrospectives/screenshots/2026-03-29-09-20.ngyj/result.png" +playwright save "result.png" +playwright save ".sgai/retrospectives/screenshots/2026-03-29-09-20.ngyj/result.md" tmux kill-session -t webserver ``` -Why it is wrong: it uses a generic session name, recreates blindly, writes to an improvised evidence path, and tears down the session instead of preserving a reusable browser-testing loop. +Why it is wrong: it uses a generic session name, recreates blindly, writes one artifact to the repo root and another to an improvised evidence path, and tears down the session instead of preserving a reusable browser-testing loop. ## Checklist @@ -148,5 +169,7 @@ Before completing, verify: - [ ] I did not use `webserver`, `testserver`, or other generic session names. - [ ] I preflighted the browser-evidence output directory before the first Playwright save. - [ ] If `.sgai/PROJECT_MANAGEMENT.md` declared a retrospective session, I used that session's `screenshots/` directory exactly. +- [ ] I prefixed every Playwright `filename` with the resolved repo-relative browser-evidence directory. +- [ ] I did not pass any bare Playwright filename basenames. - [ ] I captured snapshots and screenshots for browser evidence. - [ ] I only killed the tmux session during explicit cleanup. diff --git a/cmd/sgai/state_watcher.go b/cmd/sgai/state_watcher.go index 8d90902..d208b3c 100644 --- a/cmd/sgai/state_watcher.go +++ b/cmd/sgai/state_watcher.go @@ -4,24 +4,16 @@ import ( "context" "crypto/sha256" "encoding/hex" - "encoding/json" "fmt" "os" "path/filepath" "time" - - "github.com/ucirello/sgai/pkg/state" ) type workspaceStateSnapshot struct { - modTime time.Time - status string - needsInput bool - progressLen int - todosHash string - messagesHash string - goalModTime time.Time - goalHash string + modTime time.Time + goalModTime time.Time + goalHash string } func (s *Server) startStateWatcher() { @@ -82,40 +74,44 @@ func (s *Server) checkWorkspaceState(dir string, snapshots map[string]workspaceS } prev, hasPrev := snapshots[dir] - if hasPrev && info.ModTime().Equal(prev.modTime) { - goalChanged := false - if goalInfo != nil { - goalChanged = !goalInfo.ModTime().Equal(prev.goalModTime) - } - if !goalChanged { - return - } + if hasPrev && stateWatcherSnapshotUnchanged(prev, info.ModTime(), goalInfo) { + return } - wfState := s.workspaceCoordinator(dir).State() - - current := buildStateSnapshot(info.ModTime(), &wfState, goalInfo) + current := buildStateSnapshot(info.ModTime(), goalInfo) if !hasPrev { snapshots[dir] = current return } - s.emitStateChangeEvents(&prev, ¤t) + s.emitStateChangeEvents(dir, s.sessionRunning(dir), prev, current) snapshots[dir] = current } -func buildStateSnapshot(modTime time.Time, wfState *state.Workflow, goalInfo os.FileInfo) workspaceStateSnapshot { +func stateWatcherSnapshotUnchanged(prev workspaceStateSnapshot, modTime time.Time, goalInfo os.FileInfo) bool { + if !modTime.Equal(prev.modTime) { + return false + } + return goalSnapshotUnchanged(prev, goalInfo) +} + +func goalSnapshotUnchanged(prev workspaceStateSnapshot, goalInfo os.FileInfo) bool { + if goalInfo == nil { + return prev.goalHash == "" + } + if !goalInfo.ModTime().Equal(prev.goalModTime) { + return false + } + return hashGoalFile(goalInfo) == prev.goalHash +} + +func buildStateSnapshot(modTime time.Time, goalInfo os.FileInfo) workspaceStateSnapshot { snapshot := workspaceStateSnapshot{ - modTime: modTime, - status: wfState.Status, - needsInput: wfState.NeedsHumanInput(), - progressLen: len(wfState.Progress), - todosHash: hashTodos(wfState.ProjectTodos, wfState.Todos), - messagesHash: hashMessages(wfState.Messages), - goalModTime: time.Time{}, - goalHash: "", + modTime: modTime, + goalModTime: time.Time{}, + goalHash: "", } if goalInfo != nil { snapshot.goalModTime = goalInfo.ModTime() @@ -124,40 +120,15 @@ func buildStateSnapshot(modTime time.Time, wfState *state.Workflow, goalInfo os. return snapshot } -func (s *Server) emitStateChangeEvents(prev, current *workspaceStateSnapshot) { - changed := prev.status != current.status || - prev.needsInput != current.needsInput || - current.progressLen > prev.progressLen || - prev.todosHash != current.todosHash || - prev.messagesHash != current.messagesHash || - prev.goalHash != current.goalHash - - if changed { - s.notifyStateChange() +func (s *Server) emitStateChangeEvents(workspacePath string, running bool, prev, current workspaceStateSnapshot) { + if prev.goalHash != current.goalHash { + s.notifyWorkspaceListChange(workspacePath) + return } -} - -func hashTodos(projectTodos, agentTodos []state.TodoItem) string { - h := sha256.New() - data, errMarshal := json.Marshal(struct { - Project []state.TodoItem `json:"p"` - Agent []state.TodoItem `json:"a"` - }{projectTodos, agentTodos}) - if errMarshal != nil { - return "" + if !running && !prev.modTime.Equal(current.modTime) { + wfState := s.loadWorkspaceState(workspacePath) + s.notifyWorkspaceChangeForState(workspacePath, &wfState, false) } - h.Write(data) - return hex.EncodeToString(h.Sum(nil))[:16] -} - -func hashMessages(messages []state.Message) string { - h := sha256.New() - data, errMarshal := json.Marshal(messages) - if errMarshal != nil { - return "" - } - h.Write(data) - return hex.EncodeToString(h.Sum(nil))[:16] } func hashGoalFile(goalInfo os.FileInfo) string { diff --git a/cmd/sgai/state_watcher_test.go b/cmd/sgai/state_watcher_test.go index 9753511..091dc59 100644 --- a/cmd/sgai/state_watcher_test.go +++ b/cmd/sgai/state_watcher_test.go @@ -11,538 +11,186 @@ import ( "github.com/ucirello/sgai/pkg/state" ) -func snapshotWith(update func(*workspaceStateSnapshot)) workspaceStateSnapshot { - snapshot := workspaceStateSnapshot{ - modTime: time.Time{}, - status: "", - needsInput: false, - progressLen: 0, - todosHash: "", - messagesHash: "", - goalModTime: time.Time{}, - goalHash: "", - } - if update != nil { - update(&snapshot) - } - return snapshot -} - -func TestHashTodos(t *testing.T) { - tests := []struct { - name string - projectTodos []state.TodoItem - agentTodos []state.TodoItem - }{ - { - name: "emptyTodos", - projectTodos: []state.TodoItem{}, - agentTodos: []state.TodoItem{}, - }, - { - name: "projectTodosOnly", - projectTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 1" - item.Status = "pending" - item.Priority = "high" - }), - }, - agentTodos: []state.TodoItem{}, - }, - { - name: "agentTodosOnly", - projectTodos: []state.TodoItem{}, - agentTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 2" - item.Status = "completed" - item.Priority = "low" - }), - }, - }, - { - name: "bothTodos", - projectTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 1" - item.Status = "pending" - item.Priority = "high" - }), - }, - agentTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 2" - item.Status = "completed" - item.Priority = "low" - }), - }, - }, - { - name: "multipleTodos", - projectTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 1" - item.Status = "pending" - item.Priority = "high" - }), - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 2" - item.Status = "in_progress" - item.Priority = "medium" - }), - }, - agentTodos: []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 3" - item.Status = "completed" - item.Priority = "low" - }), - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hashTodos(tt.projectTodos, tt.agentTodos) - assert.Len(t, result, 16) - assert.NotEmpty(t, result) - }) - } -} - -func TestHashTodosConsistency(t *testing.T) { - todos := []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 1" - item.Status = "pending" - item.Priority = "high" - }), - } - - hash1 := hashTodos(todos, []state.TodoItem{}) - hash2 := hashTodos(todos, []state.TodoItem{}) - - assert.Equal(t, hash1, hash2, "same input should produce same hash") -} - -func TestHashTodosDifferent(t *testing.T) { - todos1 := []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 1" - item.Status = "pending" - item.Priority = "high" - }), - } - todos2 := []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "Task 2" - item.Status = "pending" - item.Priority = "high" - }), - } - - hash1 := hashTodos(todos1, []state.TodoItem{}) - hash2 := hashTodos(todos2, []state.TodoItem{}) - - assert.NotEqual(t, hash1, hash2, "different input should produce different hash") -} - -func TestHashMessages(t *testing.T) { - tests := []struct { - name string - messages []state.Message - }{ - { - name: "emptyMessages", - messages: []state.Message{}, - }, - { - name: "singleMessage", - messages: []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "Hello" - }), - }, - }, - { - name: "multipleMessages", - messages: []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "Hello" - }), - messageWith(func(message *state.Message) { - message.ID = 2 - message.FromAgent = "agent2" - message.ToAgent = "agent1" - message.Body = "World" - }), - }, - }, - { - name: "messageWithReadStatus", - messages: []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "Hello" - message.Read = true - }), - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hashMessages(tt.messages) - assert.Len(t, result, 16) - assert.NotEmpty(t, result) - }) - } -} - -func TestHashMessagesConsistency(t *testing.T) { - messages := []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "Hello" - }), - } - - hash1 := hashMessages(messages) - hash2 := hashMessages(messages) - - assert.Equal(t, hash1, hash2, "same input should produce same hash") -} - -func TestHashMessagesDifferent(t *testing.T) { - messages1 := []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "Hello" - }), - } - messages2 := []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "agent1" - message.ToAgent = "agent2" - message.Body = "World" - }), - } - - hash1 := hashMessages(messages1) - hash2 := hashMessages(messages2) - - assert.NotEqual(t, hash1, hash2, "different input should produce different hash") -} - -func TestHashGoalFile(t *testing.T) { - tests := []struct { - name string - fileInfo os.FileInfo - }{ - { - name: "nilFileInfo", - fileInfo: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hashGoalFile(tt.fileInfo) - if tt.fileInfo == nil { - assert.Empty(t, result) - } else { - assert.Len(t, result, 16) - } - }) - } -} - -func TestHashGoalFileWithFileInfo(t *testing.T) { - tmpFile, err := os.CreateTemp("", "goal_test_*") - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Remove(tmpFile.Name()) }) - - _, _ = tmpFile.WriteString("test content") - _ = tmpFile.Close() - - fileInfo, err := os.Stat(tmpFile.Name()) - if err != nil { - t.Fatal(err) - } - - result := hashGoalFile(fileInfo) - assert.Len(t, result, 16) - assert.NotEmpty(t, result) +func cacheWorkspacePageState(srv *Server, workspacePath string) { + var pageState apiWorkspaceFullState + pageState.Name = filepath.Base(workspacePath) + srv.workspacePageCache.set(workspacePath, pageState) } -func TestHashGoalFileConsistency(t *testing.T) { - tmpFile, err := os.CreateTemp("", "goal_test_*") - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Remove(tmpFile.Name()) }) - - _, _ = tmpFile.WriteString("test content") - _ = tmpFile.Close() - - fileInfo, err := os.Stat(tmpFile.Name()) - if err != nil { - t.Fatal(err) - } - - hash1 := hashGoalFile(fileInfo) - hash2 := hashGoalFile(fileInfo) - - assert.Equal(t, hash1, hash2, "same file info should produce same hash") +func hasCachedWorkspacePageState(srv *Server, workspacePath string) bool { + _, ok := srv.workspacePageCache.get(workspacePath) + return ok } -func TestHashGoalFileDifferentAfterModification(t *testing.T) { - tmpFile, err := os.CreateTemp("", "goal_test_*") - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Remove(tmpFile.Name()) }) - - _, _ = tmpFile.WriteString("test content") - _ = tmpFile.Close() - - fileInfo1, err := os.Stat(tmpFile.Name()) - if err != nil { - t.Fatal(err) - } - - hash1 := hashGoalFile(fileInfo1) - - time.Sleep(10 * time.Millisecond) - - _ = os.WriteFile(tmpFile.Name(), []byte("modified content"), 0o644) - - fileInfo2, err := os.Stat(tmpFile.Name()) - if err != nil { - t.Fatal(err) - } - - hash2 := hashGoalFile(fileInfo2) - - assert.NotEqual(t, hash1, hash2, "modified file should produce different hash") +func hasCachedWorkspaceListState(srv *Server) bool { + _, ok := srv.workspaceListCache.get("workspaces") + return ok } -func TestBuildStateSnapshot(t *testing.T) { - modTime := time.Now() - wfState := workflowWith(func(workflow *state.Workflow) { - workflow.Status = state.StatusWorking - workflow.HumanMessage = "test question" - workflow.Progress = []state.ProgressEntry{ - progressEntryWith(func(entry *state.ProgressEntry) { - entry.Agent = "coordinator" - entry.Description = "started" - }), - } - workflow.ProjectTodos = []state.TodoItem{ - todoItemWith(func(item *state.TodoItem) { - item.Content = "task1" - item.Status = "pending" - }), - } - workflow.Messages = []state.Message{ - messageWith(func(message *state.Message) { - message.ID = 1 - message.FromAgent = "dev" - message.ToAgent = "coord" - message.Body = "done" - }), - } - }) - - snapshot := buildStateSnapshot(modTime, &wfState, nil) - assert.Equal(t, modTime, snapshot.modTime) - assert.Equal(t, state.StatusWorking, snapshot.status) - assert.True(t, snapshot.needsInput) - assert.Equal(t, 1, snapshot.progressLen) - assert.NotEmpty(t, snapshot.todosHash) - assert.NotEmpty(t, snapshot.messagesHash) - assert.Empty(t, snapshot.goalHash) +func setLaterFileModTime(t *testing.T, path string, modTime time.Time) { + t.Helper() + require.NoError(t, os.Chtimes(path, modTime, modTime)) } -func TestBuildStateSnapshotWithGoalInfo(t *testing.T) { - tmpFile, err := os.CreateTemp("", "goal_test_*") - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Remove(tmpFile.Name()) }) - _, _ = tmpFile.WriteString("# Goal") - _ = tmpFile.Close() - - goalInfo, err := os.Stat(tmpFile.Name()) - if err != nil { - t.Fatal(err) - } +func TestCheckWorkspaceStateWithoutStateFile(t *testing.T) { + srv, _ := setupTestServer(t) + dir := t.TempDir() + snapshots := make(map[string]workspaceStateSnapshot) + activeWorkspaces := make(map[string]bool) - modTime := time.Now() - wfState := newTestWorkflow() - snapshot := buildStateSnapshot(modTime, &wfState, goalInfo) - assert.NotEmpty(t, snapshot.goalHash) - assert.False(t, snapshot.goalModTime.IsZero()) -} + srv.checkWorkspaceState(dir, snapshots, activeWorkspaces) -func TestEmitStateChangeEvents(t *testing.T) { - server, _ := setupTestServer(t) - - t.Run("noChange", func(_ *testing.T) { - prev := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.status = state.StatusWorking - snapshot.progressLen = 5 - snapshot.todosHash = "abc" - }) - server.emitStateChangeEvents(&prev, &prev) - }) - - t.Run("statusChange", func(_ *testing.T) { - prev := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.status = state.StatusWorking - }) - current := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.status = state.StatusComplete - }) - server.emitStateChangeEvents(&prev, ¤t) - }) - - t.Run("needsInputChange", func(_ *testing.T) { - prev := snapshotWith(nil) - current := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.needsInput = true - }) - server.emitStateChangeEvents(&prev, ¤t) - }) - - t.Run("progressChange", func(_ *testing.T) { - prev := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.progressLen = 5 - }) - current := snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.progressLen = 6 - }) - server.emitStateChangeEvents(&prev, ¤t) - }) + assert.True(t, activeWorkspaces[dir]) + assert.NotContains(t, snapshots, dir) } -func TestCheckWorkspaceStateSecondVisitNoChange(t *testing.T) { +func TestCheckWorkspaceStateStoresSnapshotForWorkspaceStateFile(t *testing.T) { srv, _ := setupTestServer(t) dir := t.TempDir() - sgaiDir := filepath.Join(dir, ".sgai") - require.NoError(t, os.MkdirAll(sgaiDir, 0o755)) - sp := filepath.Join(sgaiDir, "state.json") - _, errCoord := state.NewCoordinatorWith(sp, workflowWith(func(workflow *state.Workflow) { + stateFile := statePath(dir) + require.NoError(t, os.MkdirAll(filepath.Dir(stateFile), 0o755)) + _, errCoord := state.NewCoordinatorWith(stateFile, workflowWith(func(workflow *state.Workflow) { workflow.Status = state.StatusComplete })) require.NoError(t, errCoord) snapshots := make(map[string]workspaceStateSnapshot) - active := make(map[string]bool) + activeWorkspaces := make(map[string]bool) - srv.checkWorkspaceState(dir, snapshots, active) - assert.Contains(t, snapshots, dir) + srv.checkWorkspaceState(dir, snapshots, activeWorkspaces) - srv.checkWorkspaceState(dir, snapshots, active) + assert.True(t, activeWorkspaces[dir]) assert.Contains(t, snapshots, dir) + assert.False(t, snapshots[dir].modTime.IsZero()) } -func TestCheckWorkspaceStateWithMapChanges(t *testing.T) { +func TestCheckWorkspaceStateInvalidatesWorkspacePageCacheForInactiveStateChanges(t *testing.T) { srv, rootDir := setupTestServer(t) - wsDir := setupTestWorkspace(t, srv, rootDir, "cwsc-ws") - sp := filepath.Join(wsDir, ".sgai", "state.json") - _, errCoord := state.NewCoordinatorWith(sp, workflowWith(func(workflow *state.Workflow) { - workflow.Status = state.StatusComplete - workflow.Task = "all done" + wsDir := setupTestWorkspace(t, srv, rootDir, "watcher-inactive") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Goal"), 0o644)) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Task = "step 1" })) - require.NoError(t, errCoord) snapshots := make(map[string]workspaceStateSnapshot) activeWorkspaces := make(map[string]bool) + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + cacheWorkspacePageState(srv, wsDir) + + info, errStat := os.Stat(statePath(wsDir)) + require.NoError(t, errStat) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusComplete + workflow.Task = "step 2" + })) + setLaterFileModTime(t, statePath(wsDir), info.ModTime().Add(time.Second)) srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) - assert.True(t, activeWorkspaces[wsDir]) - assert.Contains(t, snapshots, wsDir) - assert.Equal(t, string(state.StatusComplete), snapshots[wsDir].status) + + assert.False(t, hasCachedWorkspacePageState(srv, wsDir)) } -func TestBuildStateSnapshotWithGoalHash(t *testing.T) { - dir := t.TempDir() - sgaiDir := filepath.Join(dir, ".sgai") - require.NoError(t, os.MkdirAll(sgaiDir, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, "GOAL.md"), []byte("# Goal"), 0o644)) - sp := filepath.Join(sgaiDir, "state.json") - _, errCoord := state.NewCoordinatorWith(sp, workflowWith(func(workflow *state.Workflow) { - workflow.Status = state.StatusComplete +func TestCheckWorkspaceStateKeepsWorkspaceListCacheForInactivePageOnlyChanges(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "watcher-page-only") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Goal"), 0o644)) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Task = "stable summary" })) - require.NoError(t, errCoord) - srv, _ := setupTestServer(t) snapshots := make(map[string]workspaceStateSnapshot) activeWorkspaces := make(map[string]bool) - srv.checkWorkspaceState(dir, snapshots, activeWorkspaces) + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + _ = srv.loadWorkspaceListResponse() + _, errLoad := srv.loadWorkspacePageState(wsDir) + require.NoError(t, errLoad) - snap, exists := snapshots[dir] - assert.True(t, exists) - assert.NotEmpty(t, snap.goalHash) + info, errStat := os.Stat(statePath(wsDir)) + require.NoError(t, errStat) + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Task = "stable summary" + workflow.Messages = []state.Message{messageWith(func(message *state.Message) { + message.ID = 1 + message.FromAgent = "go-developer" + message.ToAgent = "coordinator" + message.Body = "page-only change" + })} + })) + setLaterFileModTime(t, statePath(wsDir), info.ModTime().Add(time.Second)) + + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + + assert.True(t, hasCachedWorkspaceListState(srv)) + assert.False(t, hasCachedWorkspacePageState(srv, wsDir)) } -func TestPollWorkspaceStatesWithMultipleWorkspaces(t *testing.T) { +func TestCheckWorkspaceStateKeepsWorkspacePageCacheForRunningSessionWorkflowChanges(t *testing.T) { srv, rootDir := setupTestServer(t) - wsDir1 := setupTestWorkspace(t, srv, rootDir, "poll-ws1") - wsDir2 := setupTestWorkspace(t, srv, rootDir, "poll-ws2") - sp1 := filepath.Join(wsDir1, ".sgai", "state.json") - _, errCoord1 := state.NewCoordinatorWith(sp1, workflowWith(func(workflow *state.Workflow) { - workflow.Status = state.StatusComplete + wsDir := setupTestWorkspace(t, srv, rootDir, "watcher-running") + require.NoError(t, os.WriteFile(filepath.Join(wsDir, "GOAL.md"), []byte("# Goal"), 0o644)) + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { + workflow.Status = state.StatusWorking + workflow.Progress = []state.ProgressEntry{{Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 1"}} })) - require.NoError(t, errCoord1) - stopCachedSession(t, srv, wsDir2, workflowRef(func(workflow *state.Workflow) { - workflow.Status = state.StatusComplete + snapshots := make(map[string]workspaceStateSnapshot) + activeWorkspaces := make(map[string]bool) + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + cacheWorkspacePageState(srv, wsDir) + + info, errStat := os.Stat(statePath(wsDir)) + require.NoError(t, errStat) + coord := srv.workspaceCoordinator(wsDir) + require.NoError(t, coord.UpdateState(func(workflow *state.Workflow) { + workflow.Progress = append(workflow.Progress, state.ProgressEntry{Timestamp: time.Now().UTC().Format(time.RFC3339), Agent: "coordinator", Description: "step 2"}) })) - writeWorkflowStateToDisk(t, wsDir2, workflowRef(func(workflow *state.Workflow) { + setLaterFileModTime(t, statePath(wsDir), info.ModTime().Add(time.Second)) + + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + + assert.True(t, hasCachedWorkspacePageState(srv, wsDir)) +} + +func TestCheckWorkspaceStateInvalidatesWorkspacePageCacheForGoalChangesWhileRunning(t *testing.T) { + srv, rootDir := setupTestServer(t) + wsDir := setupTestWorkspace(t, srv, rootDir, "watcher-goal-running") + goalPath := filepath.Join(wsDir, "GOAL.md") + require.NoError(t, os.WriteFile(goalPath, []byte("# Goal"), 0o644)) + attachRunningSessionCoordinator(t, srv, wsDir, workflowRef(func(workflow *state.Workflow) { workflow.Status = state.StatusWorking - workflow.Task = "building" })) snapshots := make(map[string]workspaceStateSnapshot) - srv.pollWorkspaceStates(snapshots) - assert.NotEmpty(t, snapshots) - assert.Equal(t, state.StatusComplete, snapshots[wsDir1].status) - assert.Equal(t, state.StatusWorking, snapshots[wsDir2].status) - assert.Equal(t, state.StatusWorking, workflowStateFromDisk(t, wsDir2).Status) + activeWorkspaces := make(map[string]bool) + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + cacheWorkspacePageState(srv, wsDir) + + goalInfo, errStat := os.Stat(goalPath) + require.NoError(t, errStat) + require.NoError(t, os.WriteFile(goalPath, []byte("# Goal updated"), 0o644)) + setLaterFileModTime(t, goalPath, goalInfo.ModTime().Add(time.Second)) + + srv.checkWorkspaceState(wsDir, snapshots, activeWorkspaces) + + assert.False(t, hasCachedWorkspacePageState(srv, wsDir)) } -func TestPollWorkspaceStatesCleanupRemoved(t *testing.T) { +func TestPollWorkspaceStatesRemovesStaleSnapshots(t *testing.T) { srv, rootDir := setupTestServer(t) - wsDir := setupTestWorkspace(t, srv, rootDir, "poll-cleanup") - sp := filepath.Join(wsDir, ".sgai", "state.json") - _, errCoord := state.NewCoordinatorWith(sp, workflowWith(func(workflow *state.Workflow) { + wsDir := setupTestWorkspace(t, srv, rootDir, "watcher-cleanup") + writeWorkflowStateToDisk(t, wsDir, workflowRef(func(workflow *state.Workflow) { workflow.Status = state.StatusComplete })) - require.NoError(t, errCoord) - snapshots := make(map[string]workspaceStateSnapshot) - snapshots["/nonexistent/removed-ws"] = snapshotWith(func(snapshot *workspaceStateSnapshot) { - snapshot.status = "old" - }) + snapshots := map[string]workspaceStateSnapshot{ + "/nonexistent/removed-ws": {modTime: time.Now(), goalModTime: time.Time{}, goalHash: ""}, + } + srv.pollWorkspaceStates(snapshots) - _, exists := snapshots["/nonexistent/removed-ws"] - assert.False(t, exists) + + assert.Contains(t, snapshots, wsDir) + assert.NotContains(t, snapshots, "/nonexistent/removed-ws") } diff --git a/cmd/sgai/webapp/.gitignore b/cmd/sgai/webapp/.gitignore index 486e25e..be1533a 100644 --- a/cmd/sgai/webapp/.gitignore +++ b/cmd/sgai/webapp/.gitignore @@ -1,4 +1,5 @@ node_modules/ dist/ +.dev-dist/ .dev-index.html src/.dev-compiled.css diff --git a/cmd/sgai/webapp/dev.ts b/cmd/sgai/webapp/dev.ts index 81f7e43..0aff980 100644 --- a/cmd/sgai/webapp/dev.ts +++ b/cmd/sgai/webapp/dev.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, watch } from "fs"; +import { existsSync, mkdirSync, rmSync, watch } from "fs"; import { relative, resolve } from "path"; import postcss from "postcss"; import tailwindcss from "@tailwindcss/postcss"; @@ -41,6 +41,7 @@ function formatBuildError(logs: Array<{ message?: string }>): string { } async function buildDevBundle(): Promise { + rmSync(devDistDir, { recursive: true, force: true }); mkdirSync(devDistDir, { recursive: true }); const result = await Bun.build({ diff --git a/cmd/sgai/webapp/src/App.tsx b/cmd/sgai/webapp/src/App.tsx index feca55b..18385c0 100644 --- a/cmd/sgai/webapp/src/App.tsx +++ b/cmd/sgai/webapp/src/App.tsx @@ -1,5 +1,4 @@ import { Outlet } from "react-router"; -import { AppStateProvider } from "./contexts/AppStateProvider"; import { ConnectionStatusBanner } from "./components/ConnectionStatusBanner"; import { NotificationPermissionBar } from "./components/NotificationPermissionBar"; import { TooltipProvider } from "./components/ui/tooltip"; @@ -9,16 +8,14 @@ export function App() { useNotifications(); return ( - - - - -
-
- -
-
- - + + + +
+
+ +
+
+ ); } diff --git a/cmd/sgai/webapp/src/__tests__/router.test.tsx b/cmd/sgai/webapp/src/__tests__/router.test.tsx index d133047..17fe5b7 100644 --- a/cmd/sgai/webapp/src/__tests__/router.test.tsx +++ b/cmd/sgai/webapp/src/__tests__/router.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect } from "bun:test"; +import { describe, it, expect, spyOn } from "bun:test"; import { render, screen } from "@testing-library/react"; import { Navigate, RouterProvider, createMemoryRouter } from "react-router"; import { router } from "../router"; @@ -31,6 +31,7 @@ describe("router", () => { it("renders a product-safe recovery UI instead of the default developer error page", async () => { const rootRoute = router.routes[0]; + const consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {}); function Boom() { throw new Error("boom"); @@ -51,10 +52,14 @@ describe("router", () => { initialEntries: ["/boom"], }); - render(); + try { + render(); - expect(await screen.findByRole("heading", { name: "Something went wrong" })).toBeTruthy(); - expect(screen.getByText(/The workspace view hit an unexpected error/i)).toBeTruthy(); - expect(screen.queryByText("Unexpected Application Error!")).toBeNull(); + expect(await screen.findByRole("heading", { name: "Something went wrong" })).toBeTruthy(); + expect(screen.getByText(/The workspace view hit an unexpected error/i)).toBeTruthy(); + expect(screen.queryByText("Unexpected Application Error!")).toBeNull(); + } finally { + consoleErrorSpy.mockRestore(); + } }); }); diff --git a/cmd/sgai/webapp/src/components/ActionBar.tsx b/cmd/sgai/webapp/src/components/ActionBar.tsx index b1da2e4..a8eb3af 100644 --- a/cmd/sgai/webapp/src/components/ActionBar.tsx +++ b/cmd/sgai/webapp/src/components/ActionBar.tsx @@ -15,7 +15,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip import { cn } from "@/lib/utils"; import type { ApiActionEntry } from "@/types"; -export interface ActionBarProps { +interface ActionBarProps { actions: ApiActionEntry[]; isRunning: boolean; onActionClick: (action: ApiActionEntry, variables: Record) => void; diff --git a/cmd/sgai/webapp/src/components/ui/alert-dialog.tsx b/cmd/sgai/webapp/src/components/ui/alert-dialog.tsx index 69a238e..09ecafa 100644 --- a/cmd/sgai/webapp/src/components/ui/alert-dialog.tsx +++ b/cmd/sgai/webapp/src/components/ui/alert-dialog.tsx @@ -127,8 +127,6 @@ function AlertDialogCancel({ export { AlertDialog, - AlertDialogPortal, - AlertDialogOverlay, AlertDialogTrigger, AlertDialogContent, AlertDialogHeader, diff --git a/cmd/sgai/webapp/src/components/ui/badge.tsx b/cmd/sgai/webapp/src/components/ui/badge.tsx index ba6286b..d6d06c8 100644 --- a/cmd/sgai/webapp/src/components/ui/badge.tsx +++ b/cmd/sgai/webapp/src/components/ui/badge.tsx @@ -31,4 +31,3 @@ function Badge({ } export { Badge } -export type { BadgeProps } diff --git a/cmd/sgai/webapp/src/components/ui/card.tsx b/cmd/sgai/webapp/src/components/ui/card.tsx index 681ad98..e72d439 100644 --- a/cmd/sgai/webapp/src/components/ui/card.tsx +++ b/cmd/sgai/webapp/src/components/ui/card.tsx @@ -38,29 +38,6 @@ function CardTitle({ className, ...props }: React.ComponentProps<"div">) { ) } -function CardDescription({ className, ...props }: React.ComponentProps<"div">) { - return ( -
- ) -} - -function CardAction({ className, ...props }: React.ComponentProps<"div">) { - return ( -
- ) -} - function CardContent({ className, ...props }: React.ComponentProps<"div">) { return (
) { ) } -function CardFooter({ className, ...props }: React.ComponentProps<"div">) { - return ( -
- ) -} - export { Card, CardHeader, - CardFooter, CardTitle, - CardAction, - CardDescription, CardContent, } diff --git a/cmd/sgai/webapp/src/components/ui/collapsible.tsx b/cmd/sgai/webapp/src/components/ui/collapsible.tsx deleted file mode 100644 index b2daee2..0000000 --- a/cmd/sgai/webapp/src/components/ui/collapsible.tsx +++ /dev/null @@ -1,32 +0,0 @@ -import * as React from "react" -import { Collapsible as CollapsiblePrimitive } from "radix-ui" - -function Collapsible({ - ...props -}: React.ComponentProps) { - return -} - -function CollapsibleTrigger({ - ...props -}: React.ComponentProps) { - return ( - - ) -} - -function CollapsibleContent({ - ...props -}: React.ComponentProps) { - return ( - - ) -} - -export { Collapsible, CollapsibleTrigger, CollapsibleContent } diff --git a/cmd/sgai/webapp/src/components/ui/dropdown-menu.tsx b/cmd/sgai/webapp/src/components/ui/dropdown-menu.tsx deleted file mode 100644 index 67d9e5e..0000000 --- a/cmd/sgai/webapp/src/components/ui/dropdown-menu.tsx +++ /dev/null @@ -1,229 +0,0 @@ -import * as React from "react" -import { DropdownMenu as DropdownMenuPrimitive } from "radix-ui" -import { Check, ChevronRight, Circle } from "lucide-react" -import { cn } from "@/lib/utils" - -function DropdownMenu({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuTrigger({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuGroup({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuPortal({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuSub({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuRadioGroup({ ...props }: React.ComponentProps) { - return -} - -function DropdownMenuSubTrigger({ - className, - inset, - children, - ...props -}: React.ComponentProps & { - inset?: boolean -}) { - return ( - - {children} - - - ) -} - -function DropdownMenuSubContent({ - className, - ...props -}: React.ComponentProps) { - return ( - - ) -} - -function DropdownMenuContent({ - className, - sideOffset = 4, - ...props -}: React.ComponentProps) { - return ( - - - - ) -} - -function DropdownMenuItem({ - className, - inset, - variant = "default", - ...props -}: React.ComponentProps & { - inset?: boolean - variant?: "default" | "destructive" -}) { - return ( - - ) -} - -function DropdownMenuCheckboxItem({ - className, - children, - checked, - ...props -}: React.ComponentProps) { - return ( - - - - - - - {children} - - ) -} - -function DropdownMenuRadioItem({ - className, - children, - ...props -}: React.ComponentProps) { - return ( - - - - - - - {children} - - ) -} - -function DropdownMenuLabel({ - className, - inset, - ...props -}: React.ComponentProps & { - inset?: boolean -}) { - return ( - - ) -} - -function DropdownMenuSeparator({ - className, - ...props -}: React.ComponentProps) { - return ( - - ) -} - -function DropdownMenuShortcut({ - className, - ...props -}: React.ComponentProps<"span">) { - return ( - - ) -} - -export { - DropdownMenu, - DropdownMenuTrigger, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuCheckboxItem, - DropdownMenuRadioItem, - DropdownMenuLabel, - DropdownMenuSeparator, - DropdownMenuShortcut, - DropdownMenuGroup, - DropdownMenuPortal, - DropdownMenuSub, - DropdownMenuSubContent, - DropdownMenuSubTrigger, - DropdownMenuRadioGroup, -} diff --git a/cmd/sgai/webapp/src/components/ui/scroll-area.tsx b/cmd/sgai/webapp/src/components/ui/scroll-area.tsx index 9134497..009f51f 100644 --- a/cmd/sgai/webapp/src/components/ui/scroll-area.tsx +++ b/cmd/sgai/webapp/src/components/ui/scroll-area.tsx @@ -22,4 +22,3 @@ function ScrollArea({ } export { ScrollArea } -export type { ScrollAreaProps } diff --git a/cmd/sgai/webapp/src/components/ui/separator.tsx b/cmd/sgai/webapp/src/components/ui/separator.tsx index e1c6f3a..5276325 100644 --- a/cmd/sgai/webapp/src/components/ui/separator.tsx +++ b/cmd/sgai/webapp/src/components/ui/separator.tsx @@ -28,4 +28,3 @@ function Separator({ } export { Separator } -export type { SeparatorProps } diff --git a/cmd/sgai/webapp/src/components/ui/sheet.tsx b/cmd/sgai/webapp/src/components/ui/sheet.tsx index 5963090..bbefc2e 100644 --- a/cmd/sgai/webapp/src/components/ui/sheet.tsx +++ b/cmd/sgai/webapp/src/components/ui/sheet.tsx @@ -10,18 +10,6 @@ function Sheet({ ...props }: React.ComponentProps) { return } -function SheetTrigger({ - ...props -}: React.ComponentProps) { - return -} - -function SheetClose({ - ...props -}: React.ComponentProps) { - return -} - function SheetPortal({ ...props }: React.ComponentProps) { @@ -95,16 +83,6 @@ function SheetHeader({ className, ...props }: React.ComponentProps<"div">) { ) } -function SheetFooter({ className, ...props }: React.ComponentProps<"div">) { - return ( -
- ) -} - function SheetTitle({ className, ...props @@ -133,11 +111,8 @@ function SheetDescription({ export { Sheet, - SheetTrigger, - SheetClose, SheetContent, SheetHeader, - SheetFooter, SheetTitle, SheetDescription, } diff --git a/cmd/sgai/webapp/src/components/ui/sidebar.tsx b/cmd/sgai/webapp/src/components/ui/sidebar.tsx index 6a50783..92558d6 100644 --- a/cmd/sgai/webapp/src/components/ui/sidebar.tsx +++ b/cmd/sgai/webapp/src/components/ui/sidebar.tsx @@ -8,8 +8,6 @@ import { Slot } from "radix-ui" import { useIsMobile } from "@/hooks/use-mobile" import { cn } from "@/lib/utils" import { Button } from "@/components/ui/button" -import { Input } from "@/components/ui/input" -import { Separator } from "@/components/ui/separator" import { Sheet, SheetContent, @@ -17,7 +15,6 @@ import { SheetHeader, SheetTitle, } from "@/components/ui/sheet" -import { Skeleton } from "@/components/ui/skeleton" import { Tooltip, TooltipContent, @@ -304,34 +301,6 @@ function SidebarRail({ className, ...props }: React.ComponentProps<"button">) { ) } -function SidebarInset({ className, ...props }: React.ComponentProps<"main">) { - return ( -
- ) -} - -function SidebarInput({ - className, - ...props -}: React.ComponentProps) { - return ( - - ) -} - function SidebarHeader({ className, ...props }: React.ComponentProps<"div">) { return (
) { ) } -function SidebarSeparator({ - className, - ...props -}: React.ComponentProps) { - return ( - - ) -} - function SidebarContent({ className, ...props }: React.ComponentProps<"div">) { return (
) { ) } -function SidebarGroup({ className, ...props }: React.ComponentProps<"div">) { - return ( -
- ) -} - -function SidebarGroupLabel({ - className, - asChild = false, - ...props -}: React.ComponentProps<"div"> & { asChild?: boolean }) { - const Comp = asChild ? Slot.Root : "div" - - return ( - svg]:size-4 [&>svg]:shrink-0", - "group-data-[collapsible=icon]:-mt-8 group-data-[collapsible=icon]:opacity-0", - className - )} - {...props} - /> - ) -} - -function SidebarGroupAction({ - className, - asChild = false, - ...props -}: React.ComponentProps<"button"> & { asChild?: boolean }) { - const Comp = asChild ? Slot.Root : "button" - - return ( - svg]:size-4 [&>svg]:shrink-0", - // Increases the hit area of the button on mobile. - "after:absolute after:-inset-2 md:after:hidden", - "group-data-[collapsible=icon]:hidden", - className - )} - {...props} - /> - ) -} - -function SidebarGroupContent({ - className, - ...props -}: React.ComponentProps<"div">) { - return ( -
- ) -} - function SidebarMenu({ className, ...props }: React.ComponentProps<"ul">) { return (
    & { - asChild?: boolean - showOnHover?: boolean -}) { - const Comp = asChild ? Slot.Root : "button" - - return ( - svg]:size-4 [&>svg]:shrink-0", - // Increases the hit area of the button on mobile. - "after:absolute after:-inset-2 md:after:hidden", - "peer-data-[size=sm]/menu-button:top-1", - "peer-data-[size=default]/menu-button:top-1.5", - "peer-data-[size=lg]/menu-button:top-2.5", - "group-data-[collapsible=icon]:hidden", - showOnHover && - "peer-data-[active=true]/menu-button:text-sidebar-accent-foreground group-focus-within/menu-item:opacity-100 group-hover/menu-item:opacity-100 data-[state=open]:opacity-100 md:opacity-0", - className - )} - {...props} - /> - ) -} - -function SidebarMenuBadge({ - className, - ...props -}: React.ComponentProps<"div">) { - return ( -
    - ) -} - -function SidebarMenuSkeleton({ - className, - showIcon = false, - ...props -}: React.ComponentProps<"div"> & { - showIcon?: boolean -}) { - // Random width between 50 to 90%. - const width = React.useMemo(() => { - return `${Math.floor(Math.random() * 40) + 50}%` - }, []) - - return ( -
    - {showIcon && ( - - )} - -
    - ) -} - -function SidebarMenuSub({ className, ...props }: React.ComponentProps<"ul">) { - return ( -
      - ) -} - -function SidebarMenuSubItem({ - className, - ...props -}: React.ComponentProps<"li">) { - return ( -
    • - ) -} - -function SidebarMenuSubButton({ - asChild = false, - size = "md", - isActive = false, - className, - ...props -}: React.ComponentProps<"a"> & { - asChild?: boolean - size?: "sm" | "md" - isActive?: boolean -}) { - const Comp = asChild ? Slot.Root : "a" - - return ( - svg]:text-sidebar-accent-foreground flex h-7 min-w-0 -translate-x-px items-center gap-2 overflow-hidden rounded-md px-2 outline-hidden focus-visible:ring-2 disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:opacity-50 [&>span:last-child]:truncate [&>svg]:size-4 [&>svg]:shrink-0", - "data-[active=true]:bg-sidebar-accent data-[active=true]:text-sidebar-accent-foreground", - size === "sm" && "text-xs", - size === "md" && "text-sm", - "group-data-[collapsible=icon]:hidden", - className - )} - {...props} - /> - ) -} - export { Sidebar, SidebarContent, SidebarFooter, - SidebarGroup, - SidebarGroupAction, - SidebarGroupContent, - SidebarGroupLabel, SidebarHeader, - SidebarInput, - SidebarInset, SidebarMenu, - SidebarMenuAction, - SidebarMenuBadge, SidebarMenuButton, SidebarMenuItem, - SidebarMenuSkeleton, - SidebarMenuSub, - SidebarMenuSubButton, - SidebarMenuSubItem, SidebarProvider, SidebarRail, - SidebarSeparator, SidebarTrigger, useSidebar, } diff --git a/cmd/sgai/webapp/src/components/ui/switch.tsx b/cmd/sgai/webapp/src/components/ui/switch.tsx deleted file mode 100644 index ba99258..0000000 --- a/cmd/sgai/webapp/src/components/ui/switch.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import * as React from "react" - -import { cn } from "@/lib/utils" - -interface SwitchProps extends Omit, "onChange"> { - checked?: boolean - onCheckedChange?: (checked: boolean) => void -} - -function Switch({ className, checked = false, onCheckedChange, disabled, ...props }: SwitchProps) { - return ( - - ) -} - -export { Switch } diff --git a/cmd/sgai/webapp/src/components/ui/table.tsx b/cmd/sgai/webapp/src/components/ui/table.tsx index 5513a5c..7ba5a40 100644 --- a/cmd/sgai/webapp/src/components/ui/table.tsx +++ b/cmd/sgai/webapp/src/components/ui/table.tsx @@ -37,19 +37,6 @@ function TableBody({ className, ...props }: React.ComponentProps<"tbody">) { ) } -function TableFooter({ className, ...props }: React.ComponentProps<"tfoot">) { - return ( - tr]:last:border-b-0", - className - )} - {...props} - /> - ) -} - function TableRow({ className, ...props }: React.ComponentProps<"tr">) { return ( ) { ) } -function TableCaption({ - className, - ...props -}: React.ComponentProps<"caption">) { - return ( - - ) -} - export { Table, TableHeader, TableBody, - TableFooter, TableHead, TableRow, TableCell, - TableCaption, } diff --git a/cmd/sgai/webapp/src/contexts/AppStateProvider.tsx b/cmd/sgai/webapp/src/contexts/AppStateProvider.tsx deleted file mode 100644 index 3683ab3..0000000 --- a/cmd/sgai/webapp/src/contexts/AppStateProvider.tsx +++ /dev/null @@ -1,94 +0,0 @@ -import { - createContext, - useContext, - useReducer, - type ReactNode, -} from "react"; - -export interface AppState { - selectedWorkspace: string | null; - ui: { - panelCollapsed: boolean; - activeTab: string; - }; -} - -export type AppAction = - | { type: "workspace/select"; workspace: string } - | { type: "ui/togglePanel" } - | { type: "ui/setTab"; tab: string }; - -const initialState: AppState = { - selectedWorkspace: null, - ui: { - panelCollapsed: false, - activeTab: "goal", - }, -}; - -function appReducer(state: AppState, action: AppAction): AppState { - switch (action.type) { - case "workspace/select": - return { - ...state, - selectedWorkspace: action.workspace, - }; - case "ui/togglePanel": - return { - ...state, - ui: { - ...state.ui, - panelCollapsed: !state.ui.panelCollapsed, - }, - }; - case "ui/setTab": - return { - ...state, - ui: { - ...state.ui, - activeTab: action.tab, - }, - }; - default: { - const _exhaustive: never = action; - return state; - } - } -} - -const AppStateContext = createContext(undefined); -const AppDispatchContext = createContext< - React.Dispatch | undefined ->(undefined); - -interface AppStateProviderProps { - children: ReactNode; -} - -export function AppStateProvider({ children }: AppStateProviderProps) { - const [state, dispatch] = useReducer(appReducer, initialState); - - return ( - - - {children} - - - ); -} - -export function useAppState(): AppState { - const context = useContext(AppStateContext); - if (context === undefined) { - throw new Error("useAppState must be used within an AppStateProvider"); - } - return context; -} - -export function useAppDispatch(): React.Dispatch { - const context = useContext(AppDispatchContext); - if (context === undefined) { - throw new Error("useAppDispatch must be used within an AppStateProvider"); - } - return context; -} diff --git a/cmd/sgai/webapp/src/hooks/useResponseForm.ts b/cmd/sgai/webapp/src/hooks/useResponseForm.ts index 3f92670..a233257 100644 --- a/cmd/sgai/webapp/src/hooks/useResponseForm.ts +++ b/cmd/sgai/webapp/src/hooks/useResponseForm.ts @@ -1,9 +1,9 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { api, ApiError } from "@/lib/api"; -import { useFactoryState, triggerFactoryRefresh } from "@/lib/factory-state"; +import { useWorkspacePageState } from "@/lib/workspace-page-state"; import type { ApiPendingQuestionResponse, ApiWorkspaceEntry } from "@/types"; -export interface StoredResponseState { +interface StoredResponseState { selections: Record; otherText: string; promptToken: string; @@ -13,7 +13,7 @@ function getStorageKey(prefix: string, workspaceName: string): string { return `${prefix}${workspaceName}`; } -export function loadStoredState(prefix: string, workspaceName: string): StoredResponseState | null { +function loadStoredState(prefix: string, workspaceName: string): StoredResponseState | null { try { const stored = sessionStorage.getItem(getStorageKey(prefix, workspaceName)); if (stored) { @@ -25,7 +25,7 @@ export function loadStoredState(prefix: string, workspaceName: string): StoredRe return null; } -export function saveStoredState(prefix: string, workspaceName: string, state: StoredResponseState): void { +function saveStoredState(prefix: string, workspaceName: string, state: StoredResponseState): void { try { sessionStorage.setItem(getStorageKey(prefix, workspaceName), JSON.stringify(state)); } catch { @@ -33,7 +33,7 @@ export function saveStoredState(prefix: string, workspaceName: string, state: St } } -export function clearStoredState(prefix: string, workspaceName: string): void { +function clearStoredState(prefix: string, workspaceName: string): void { try { sessionStorage.removeItem(getStorageKey(prefix, workspaceName)); } catch { @@ -77,8 +77,7 @@ export function useResponseForm({ const hasUnsavedChangesRef = useRef(false); const previousPromptTokenRef = useRef(null); - const { workspaces, fetchStatus } = useFactoryState(); - const workspace = workspaces.find((ws) => ws.name === workspaceName) ?? null; + const { workspace, fetchStatus } = useWorkspacePageState(workspaceName); const question = workspace?.pendingQuestion ?? null; const promptToken = question?.promptToken ?? null; const loading = fetchStatus === "fetching" && workspace === null; @@ -175,7 +174,6 @@ export function useResponseForm({ answer: otherText.trim(), selectedChoices: allSelectedChoices, }); - triggerFactoryRefresh(); clearStoredState(storagePrefix, workspaceName); hasUnsavedChangesRef.current = false; diff --git a/cmd/sgai/webapp/src/lib/__tests__/factory-state.test.ts b/cmd/sgai/webapp/src/lib/__tests__/factory-state.test.ts index fef5aa3..5b02b10 100644 --- a/cmd/sgai/webapp/src/lib/__tests__/factory-state.test.ts +++ b/cmd/sgai/webapp/src/lib/__tests__/factory-state.test.ts @@ -1,112 +1,173 @@ -import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test"; -import { waitFor } from "@testing-library/dom"; -import { resetFactoryStateStore, triggerFactoryRefresh } from "../factory-state"; +import { describe, it, expect, beforeEach, afterEach, mock, vi } from "bun:test"; +import { act, cleanup, render } from "@testing-library/react"; +import { createElement } from "react"; +import { resetFactoryStateStore, triggerFactoryRefresh, useFactoryState } from "../factory-state"; -const mockFetch = mock(() => - Promise.resolve({ +function createResponse(body: unknown): Response { + return { ok: true, status: 200, - json: () => Promise.resolve({ workspaces: [] }), - } as Response) + json: () => Promise.resolve(body), + } as Response; +} + +type EventListenerMap = Map) => void>>; + +class MockEventSource { + static instances: MockEventSource[] = []; + + onopen: ((this: EventSource, event: Event) => void) | null = null; + onerror: ((this: EventSource, event: Event) => void) | null = null; + + private listeners: EventListenerMap = new Map(); + + constructor(public readonly url: string) { + MockEventSource.instances.push(this); + } + + addEventListener(type: string, listener: (event: MessageEvent) => void): void { + const listeners = this.listeners.get(type) ?? new Set(); + listeners.add(listener); + this.listeners.set(type, listeners); + } + + close(): void {} + + emit(type: string, payload: unknown): void { + const event = { data: JSON.stringify(payload) } as MessageEvent; + for (const listener of this.listeners.get(type) ?? []) { + listener(event); + } + } + + static reset(): void { + MockEventSource.instances = []; + } +} + +async function advanceTimersAndFlush(timeMs: number): Promise { + await act(async () => { + vi.advanceTimersByTime(timeMs); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +async function emitSignalAndFlush(source: MockEventSource, type: string, payload: unknown): Promise { + await act(async () => { + source.emit(type, payload); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +const mockFetch = mock(() => + Promise.resolve(createResponse({ workspaces: [{ name: "test-workspace", dir: "/tmp/test-workspace" }] })) ); -const REFRESH_TIMEOUT_MS = 5000; +function FactoryStateProbe() { + const { workspaces, fetchStatus } = useFactoryState(); + + return createElement( + "div", + null, + createElement("span", { "data-testid": "workspace-count" }, String(workspaces.length)), + createElement("span", { "data-testid": "fetch-status" }, fetchStatus), + ); +} beforeEach(() => { globalThis.fetch = mockFetch as unknown as typeof fetch; + globalThis.EventSource = MockEventSource as unknown as typeof EventSource; mockFetch.mockClear(); mockFetch.mockImplementation(() => - Promise.resolve({ - ok: true, - status: 200, - json: () => Promise.resolve({ workspaces: [] }), - } as Response) + Promise.resolve(createResponse({ workspaces: [{ name: "test-workspace", dir: "/tmp/test-workspace" }] })) ); + MockEventSource.reset(); }); afterEach(() => { + cleanup(); resetFactoryStateStore(); + MockEventSource.reset(); + vi.useRealTimers(); }); describe("factory-state store", () => { - describe("resetFactoryStateStore", () => { - it("resets the store without errors", () => { - expect(() => resetFactoryStateStore()).not.toThrow(); - }); - - it("can be called multiple times safely", () => { - resetFactoryStateStore(); - resetFactoryStateStore(); - resetFactoryStateStore(); - }); - }); + it("ignores page-only workspace SSE events and refreshes on reload events", async () => { + vi.useFakeTimers(); - describe("triggerFactoryRefresh", () => { - it("triggers a refresh without errors", () => { - expect(() => triggerFactoryRefresh()).not.toThrow(); - }); - - it("calls fetch to /api/v1/state", async () => { - triggerFactoryRefresh(); - await waitFor(() => { - const stateCalls = mockFetch.mock.calls.filter( - (call) => (call[0] as string) === "/api/v1/state" - ); - expect(stateCalls.length).toBeGreaterThan(0); - }, { timeout: REFRESH_TIMEOUT_MS }); - }); - }); + render(createElement(FactoryStateProbe)); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + + const source = MockEventSource.instances[0]; + expect(source).toBeDefined(); - describe("useFactoryState", () => { - it("exports useFactoryState function", async () => { - const mod = await import("../factory-state"); - expect(typeof mod.useFactoryState).toBe("function"); - }); + source.onopen?.call(source as unknown as EventSource, new Event("open")); + + await emitSignalAndFlush(source, "workspace", { workspace: "test-workspace" }); + + expect(mockFetch.mock.calls.length).toBe(1); + + await emitSignalAndFlush(source, "reload", { workspace: "test-workspace" }); + + expect(mockFetch.mock.calls.length).toBe(2); }); - describe("FetchStatus type", () => { - it("exports FetchStatus type (verified by TypeScript compilation)", () => { - const status: import("../factory-state").FetchStatus = "idle"; - expect(status).toBe("idle"); - }); + it("stops visible polling once SSE is connected and resumes fallback polling after disconnect", async () => { + vi.useFakeTimers(); + + render(createElement(FactoryStateProbe)); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + + const source = MockEventSource.instances[0]; + expect(source).toBeDefined(); + + source.onopen?.call(source as unknown as EventSource, new Event("open")); + + await advanceTimersAndFlush(3000); + expect(mockFetch.mock.calls.length).toBe(1); + + source.onerror?.call(source as unknown as EventSource, new Event("error")); + + await advanceTimersAndFlush(3000); + expect(mockFetch.mock.calls.length).toBe(2); }); - describe("FactoryStateSnapshot type", () => { - it("has correct shape", () => { - const snapshot: import("../factory-state").FactoryStateSnapshot = { - workspaces: [], - fetchStatus: "idle", - lastFetchedAt: null, - }; - expect(snapshot.workspaces).toEqual([]); - expect(snapshot.fetchStatus).toBe("idle"); - expect(snapshot.lastFetchedAt).toBeNull(); - }); + it("does not fall back to visible polling while an idle EventSource still exists", async () => { + vi.useFakeTimers(); + + render(createElement(FactoryStateProbe)); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + expect(MockEventSource.instances.length).toBe(1); + + await advanceTimersAndFlush(9000); + + expect(mockFetch.mock.calls.length).toBe(1); + expect(MockEventSource.instances.length).toBe(1); }); - describe("error handling", () => { - it("handles fetch failure gracefully", async () => { - mockFetch.mockImplementation(() => Promise.reject(new Error("Network error"))); - - expect(() => triggerFactoryRefresh()).not.toThrow(); - await waitFor(() => { - expect(mockFetch.mock.calls.length).toBeGreaterThan(0); - }, { timeout: REFRESH_TIMEOUT_MS }); - }); - - it("handles non-ok response gracefully", async () => { - mockFetch.mockImplementation(() => - Promise.resolve({ - ok: false, - status: 500, - json: () => Promise.reject(new Error("no json")), - } as Response) - ); - - expect(() => triggerFactoryRefresh()).not.toThrow(); - await waitFor(() => { - expect(mockFetch.mock.calls.length).toBeGreaterThan(0); - }, { timeout: REFRESH_TIMEOUT_MS }); - }); + it("stops polling and ignores refresh requests after the last subscriber unsubscribes", async () => { + vi.useFakeTimers(); + + const { unmount } = render(createElement(FactoryStateProbe)); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + + unmount(); + + triggerFactoryRefresh(); + + await advanceTimersAndFlush(300); + await advanceTimersAndFlush(3000); + + expect(mockFetch.mock.calls.length).toBe(1); }); }); 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 new file mode 100644 index 0000000..f2302b0 --- /dev/null +++ b/cmd/sgai/webapp/src/lib/__tests__/workspace-page-state.test.ts @@ -0,0 +1,259 @@ +import { describe, it, expect, beforeEach, afterEach, mock, vi } from "bun:test"; +import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import { createElement } from "react"; +import { + resetWorkspacePageStateStores, + triggerWorkspacePageRefresh, + useWorkspacePageState, +} from "../workspace-page-state"; + +function deferredValue() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +function createResponse(body: unknown): Response { + return { + ok: true, + status: 200, + json: () => Promise.resolve(body), + } as Response; +} + +type EventListenerMap = Map) => void>>; + +class MockEventSource { + static instances: MockEventSource[] = []; + + onopen: ((this: EventSource, event: Event) => void) | null = null; + onerror: ((this: EventSource, event: Event) => void) | null = null; + + private listeners: EventListenerMap = new Map(); + + constructor(public readonly url: string) { + MockEventSource.instances.push(this); + } + + addEventListener(type: string, listener: (event: MessageEvent) => void): void { + const listeners = this.listeners.get(type) ?? new Set(); + listeners.add(listener); + this.listeners.set(type, listeners); + } + + close(): void {} + + emit(type: string, payload: unknown): void { + const event = { data: JSON.stringify(payload) } as MessageEvent; + for (const listener of this.listeners.get(type) ?? []) { + listener(event); + } + } + + static reset(): void { + MockEventSource.instances = []; + } +} + +async function advanceTimersAndFlush(timeMs: number): Promise { + await act(async () => { + vi.advanceTimersByTime(timeMs); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +const mockFetch = mock(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" }))); + +function WorkspacePageProbe({ workspaceName }: { workspaceName: string }) { + const { workspace, fetchStatus } = useWorkspacePageState(workspaceName); + + return createElement( + "div", + null, + createElement("span", { "data-testid": "workspace-name" }, workspace?.name ?? ""), + createElement("span", { "data-testid": "fetch-status" }, fetchStatus), + ); +} + +beforeEach(() => { + globalThis.fetch = mockFetch as unknown as typeof fetch; + globalThis.EventSource = MockEventSource as unknown as typeof EventSource; + mockFetch.mockClear(); + mockFetch.mockImplementation(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" }))); + MockEventSource.reset(); +}); + +afterEach(() => { + cleanup(); + resetWorkspacePageStateStores(); + MockEventSource.reset(); + vi.useRealTimers(); +}); + +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" })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); + }); + + mockFetch.mockClear(); + mockFetch + .mockImplementationOnce(() => firstResponse.promise) + .mockImplementationOnce(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace", status: "fresh" }))); + + vi.useFakeTimers(); + + triggerWorkspacePageRefresh("test-workspace"); + await advanceTimersAndFlush(300); + + expect(mockFetch.mock.calls.length).toBe(1); + + triggerWorkspacePageRefresh("test-workspace"); + await advanceTimersAndFlush(300); + expect(mockFetch.mock.calls.length).toBe(1); + + firstResponse.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" })); + + await waitFor(() => { + expect(mockFetch.mock.calls.length).toBe(2); + }); + }); + + it("keeps the existing snapshot idle during a background refresh", async () => { + const secondResponse = deferredValue(); + + mockFetch + .mockImplementationOnce(() => Promise.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace" }))) + .mockImplementationOnce(() => secondResponse.promise); + + render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); + expect(screen.getByTestId("fetch-status").textContent).toBe("idle"); + }); + + triggerWorkspacePageRefresh("test-workspace"); + + await waitFor(() => { + expect(mockFetch.mock.calls.length).toBe(2); + }); + + expect(screen.getByTestId("fetch-status").textContent).toBe("idle"); + + secondResponse.resolve(createResponse({ name: "test-workspace", dir: "/tmp/test-workspace", status: "updated" })); + + await waitFor(() => { + expect(screen.getByTestId("fetch-status").textContent).toBe("idle"); + }); + }); + + it("stops polling and ignores refresh requests after the last subscriber unsubscribes", async () => { + vi.useFakeTimers(); + + const { unmount } = render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); + }); + + expect(mockFetch.mock.calls.length).toBe(1); + + unmount(); + + triggerWorkspacePageRefresh("test-workspace"); + + await advanceTimersAndFlush(300); + await advanceTimersAndFlush(3000); + + expect(mockFetch.mock.calls.length).toBe(1); + }); + + it("ignores generic reload SSE events after the workspace page has loaded", async () => { + render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); + }); + + const source = MockEventSource.instances[0]; + expect(source).toBeDefined(); + + source.emit("reload", {}); + expect(mockFetch.mock.calls.length).toBe(1); + + source.emit("workspace", { workspace: "/tmp/test-workspace" }); + + await waitFor(() => { + expect(mockFetch.mock.calls.length).toBe(2); + }); + }); + + it("refreshes only for matching workspace-dir SSE events and keeps state fetches headerless", async () => { + render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + + await waitFor(() => { + expect(screen.getByTestId("workspace-name").textContent).toBe("test-workspace"); + }); + + const source = MockEventSource.instances[0]; + expect(source).toBeDefined(); + + source.emit("workspace", { workspace: "/tmp/other-workspace" }); + expect(mockFetch.mock.calls.length).toBe(1); + + source.emit("workspace", { workspace: "/tmp/test-workspace" }); + + await waitFor(() => { + expect(mockFetch.mock.calls.length).toBe(2); + }); + + expect(mockFetch.mock.calls[1]).toHaveLength(1); + }); + + it("stops visible polling once SSE is connected and resumes fallback polling after disconnect", async () => { + vi.useFakeTimers(); + + render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + + const source = MockEventSource.instances[0]; + expect(source).toBeDefined(); + + source.onopen?.call(source as unknown as EventSource, new Event("open")); + + await advanceTimersAndFlush(3000); + expect(mockFetch.mock.calls.length).toBe(1); + + source.onerror?.call(source as unknown as EventSource, new Event("error")); + + await advanceTimersAndFlush(3000); + expect(mockFetch.mock.calls.length).toBe(2); + }); + + it("does not fall back to visible polling while an idle EventSource still exists", async () => { + vi.useFakeTimers(); + + render(createElement(WorkspacePageProbe, { workspaceName: "test-workspace" })); + await advanceTimersAndFlush(0); + + expect(mockFetch.mock.calls.length).toBe(1); + expect(MockEventSource.instances.length).toBe(1); + + await advanceTimersAndFlush(9000); + + expect(mockFetch.mock.calls.length).toBe(1); + expect(MockEventSource.instances.length).toBe(1); + }); +}); diff --git a/cmd/sgai/webapp/src/lib/factory-state.ts b/cmd/sgai/webapp/src/lib/factory-state.ts index 66c6d5d..89c6199 100644 --- a/cmd/sgai/webapp/src/lib/factory-state.ts +++ b/cmd/sgai/webapp/src/lib/factory-state.ts @@ -19,13 +19,12 @@ type Listener = () => void; const POLL_INTERVAL_MS = 3000; const SLOW_POLL_INTERVAL_MS = 10000; -const SSE_CONNECT_TIMEOUT_MS = 5000; const SSE_BASE_BACKOFF_MS = 1000; const SSE_MAX_BACKOFF_MS = 30000; const DEBOUNCE_MS = 300; -function createFactoryStateStore() { +function createFactoryStateStore(removeStore: () => void) { let snapshot: FactoryStateSnapshot = { workspaces: [], fetchStatus: "idle", @@ -35,15 +34,25 @@ function createFactoryStateStore() { const listeners: Set = new Set(); let pollTimerId: ReturnType | null = null; let sseSource: EventSource | null = null; - let sseTimeoutId: ReturnType | null = null; let sseReconnectTimerId: ReturnType | null = null; let sseReconnectAttempts = 0; - let sseConnected = false; let isFetching = false; let isDestroyed = false; let isStarted = false; let refreshDebounceTimerId: ReturnType | null = null; + function isDocumentHidden(): boolean { + return typeof document !== "undefined" && document.visibilityState === "hidden"; + } + + function shouldPoll(): boolean { + return isDocumentHidden() || sseSource === null; + } + + function pollIntervalMs(): number { + return isDocumentHidden() ? SLOW_POLL_INTERVAL_MS : POLL_INTERVAL_MS; + } + function emitChange() { for (const listener of listeners) { listener(); @@ -61,7 +70,7 @@ function createFactoryStateStore() { updateSnapshot({ fetchStatus: "fetching" }); try { - const response = await fetch("/api/v1/state"); + const response = await fetch("/api/v1/workspaces"); if (isDestroyed) return; if (!response.ok) { updateSnapshot({ fetchStatus: "error" }); @@ -83,28 +92,28 @@ function createFactoryStateStore() { } } - function schedulePoll(intervalMs: number) { + function syncPolling() { if (pollTimerId !== null) { clearTimeout(pollTimerId); pollTimerId = null; } - if (isDestroyed) return; + if (isDestroyed || !shouldPoll()) return; pollTimerId = setTimeout(() => { pollTimerId = null; - const hidden = typeof document !== "undefined" && document.visibilityState === "hidden"; void fetchState(); - schedulePoll(hidden ? SLOW_POLL_INTERVAL_MS : POLL_INTERVAL_MS); - }, intervalMs); + syncPolling(); + }, pollIntervalMs()); } function handleVisibilityChange() { if (isDestroyed) return; - if (document.visibilityState === "visible") { + if (!isDocumentHidden()) { void fetchState(); - schedulePoll(POLL_INTERVAL_MS); - } else { - schedulePoll(SLOW_POLL_INTERVAL_MS); + if (sseSource === null && sseReconnectTimerId === null) { + connectSSESignal(); + } } + syncPolling(); } function sseBackoffDelay(): number { @@ -126,26 +135,13 @@ function createFactoryStateStore() { } function connectSSESignal() { - if (sseSource !== null || isDestroyed) return; + if (sseSource !== null || isDestroyed || typeof EventSource === "undefined") return; sseSource = new EventSource("/api/v1/signal"); - sseTimeoutId = setTimeout(() => { - sseTimeoutId = null; - if (!sseConnected && sseSource !== null) { - sseSource.close(); - sseSource = null; - scheduleSSEReconnect(); - } - }, SSE_CONNECT_TIMEOUT_MS); - sseSource.onopen = () => { - sseConnected = true; sseReconnectAttempts = 0; - if (sseTimeoutId !== null) { - clearTimeout(sseTimeoutId); - sseTimeoutId = null; - } + syncPolling(); }; sseSource.onerror = () => { @@ -153,19 +149,17 @@ function createFactoryStateStore() { sseSource.close(); sseSource = null; } - if (sseTimeoutId !== null) { - clearTimeout(sseTimeoutId); - sseTimeoutId = null; - } - sseConnected = false; + syncPolling(); scheduleSSEReconnect(); }; - sseSource.addEventListener("reload", () => { + const handleReloadSignal = () => { if (!isDestroyed) { void fetchState(); } - }); + }; + + sseSource.addEventListener("reload", handleReloadSignal); } function start() { @@ -173,8 +167,8 @@ function createFactoryStateStore() { isStarted = true; void fetchState(); - schedulePoll(POLL_INTERVAL_MS); connectSSESignal(); + syncPolling(); if (typeof document !== "undefined") { document.addEventListener("visibilitychange", handleVisibilityChange); @@ -192,11 +186,15 @@ function createFactoryStateStore() { refreshDebounceTimerId = setTimeout(() => { refreshDebounceTimerId = null; void fetchState(); - schedulePoll(POLL_INTERVAL_MS); + syncPolling(); }, DEBOUNCE_MS); } function stop() { + if (isDestroyed) { + return; + } + isDestroyed = true; isStarted = false; @@ -210,11 +208,6 @@ function createFactoryStateStore() { refreshDebounceTimerId = null; } - if (sseTimeoutId !== null) { - clearTimeout(sseTimeoutId); - sseTimeoutId = null; - } - if (sseReconnectTimerId !== null) { clearTimeout(sseReconnectTimerId); sseReconnectTimerId = null; @@ -228,6 +221,9 @@ function createFactoryStateStore() { if (typeof document !== "undefined") { document.removeEventListener("visibilitychange", handleVisibilityChange); } + + listeners.clear(); + removeStore(); } function subscribe(listener: Listener): () => void { @@ -237,6 +233,9 @@ function createFactoryStateStore() { } return () => { listeners.delete(listener); + if (listeners.size === 0) { + stop(); + } }; } @@ -267,7 +266,13 @@ let storeInstance: FactoryStateStore | null = null; function getFactoryStateStore(): FactoryStateStore { if (!storeInstance) { - storeInstance = createFactoryStateStore(); + let createdStore!: FactoryStateStore; + createdStore = createFactoryStateStore(() => { + if (storeInstance === createdStore) { + storeInstance = null; + } + }); + storeInstance = createdStore; } return storeInstance; } @@ -280,8 +285,7 @@ export function resetFactoryStateStore(): void { } export function triggerFactoryRefresh(): void { - const store = getFactoryStateStore(); - store.triggerRefresh(); + storeInstance?.triggerRefresh(); } export function getFactoryStateSnapshot(): FactoryStateSnapshot { diff --git a/cmd/sgai/webapp/src/lib/repository-label.ts b/cmd/sgai/webapp/src/lib/repository-label.ts deleted file mode 100644 index 38e6454..0000000 --- a/cmd/sgai/webapp/src/lib/repository-label.ts +++ /dev/null @@ -1,27 +0,0 @@ -interface RepositoryLabelSource { - name: string; - title?: string | null; -} - -function normalizeRepositoryLabelPart(value: string | null | undefined): string { - return value?.trim() ?? ""; -} - -export function resolveRepositoryLabel(source: RepositoryLabelSource): string { - const title = normalizeRepositoryLabelPart(source.title); - return title || normalizeRepositoryLabelPart(source.name); -} - -export function resolveRepositoryLabelFromCandidates( - name: string, - ...titleCandidates: Array -): string { - for (const titleCandidate of titleCandidates) { - const title = normalizeRepositoryLabelPart(titleCandidate); - if (title) { - return title; - } - } - - return normalizeRepositoryLabelPart(name); -} diff --git a/cmd/sgai/webapp/src/lib/workspace-delete-copy.ts b/cmd/sgai/webapp/src/lib/workspace-delete-copy.ts deleted file mode 100644 index a82ebc5..0000000 --- a/cmd/sgai/webapp/src/lib/workspace-delete-copy.ts +++ /dev/null @@ -1,42 +0,0 @@ -type WorkspaceDeletionCopyOptions = { - workspaceLabel: string; - isExternal?: boolean; - isFork?: boolean; -}; - -export function getWorkspaceDeletionCopy({ - workspaceLabel, - isExternal = false, - isFork = false, -}: WorkspaceDeletionCopyOptions) { - if (isExternal && !isFork) { - return { - triggerText: "Remove", - triggerLabel: `Detach ${workspaceLabel}`, - dialogTitle: "Detach workspace", - dialogDescription: `This will remove '${workspaceLabel}' from the interface. The directory and its contents will NOT be deleted.`, - confirmText: "Remove", - pendingText: "Removing...", - }; - } - - if (isFork) { - return { - triggerText: "Delete fork", - triggerLabel: `Delete fork ${workspaceLabel}`, - dialogTitle: "Delete fork", - dialogDescription: `This will permanently delete '${workspaceLabel}' from disk. This action cannot be undone.`, - confirmText: "Delete fork", - pendingText: "Deleting...", - }; - } - - return { - triggerText: "Delete workspace", - triggerLabel: `Delete workspace ${workspaceLabel}`, - dialogTitle: "Delete workspace", - dialogDescription: `This will permanently delete '${workspaceLabel}' from disk. This action cannot be undone.`, - confirmText: "Delete workspace", - pendingText: "Deleting...", - }; -} diff --git a/cmd/sgai/webapp/src/lib/workspace-identity.ts b/cmd/sgai/webapp/src/lib/workspace-identity.ts index c1bbc43..32e44ad 100644 --- a/cmd/sgai/webapp/src/lib/workspace-identity.ts +++ b/cmd/sgai/webapp/src/lib/workspace-identity.ts @@ -1,7 +1,7 @@ import { getRepositoryTitle } from "@/lib/repository-title"; import type { ApiWorkspaceEntry } from "@/types"; -export type WorkspaceIdentity = Pick; +type WorkspaceIdentity = Pick; type WorkspaceLabelSource = WorkspaceIdentity & Pick; diff --git a/cmd/sgai/webapp/src/lib/workspace-page-state.ts b/cmd/sgai/webapp/src/lib/workspace-page-state.ts new file mode 100644 index 0000000..e7747eb --- /dev/null +++ b/cmd/sgai/webapp/src/lib/workspace-page-state.ts @@ -0,0 +1,353 @@ +import { useSyncExternalStore } from "react"; +import type { ApiWorkspaceEntry } from "@/types"; +import type { FetchStatus } from "./factory-state"; + +interface WorkspacePageSnapshot { + workspace: ApiWorkspaceEntry | null; + fetchStatus: FetchStatus; + lastFetchedAt: number | null; +} + +interface SignalPayload { + workspace?: string; +} + +type Listener = () => void; + +const POLL_INTERVAL_MS = 3000; +const SLOW_POLL_INTERVAL_MS = 10000; +const SSE_BASE_BACKOFF_MS = 1000; +const SSE_MAX_BACKOFF_MS = 30000; +const DEBOUNCE_MS = 300; + +function createWorkspacePageStore(workspaceName: string, removeStore: () => void) { + let snapshot: WorkspacePageSnapshot = { + workspace: null, + fetchStatus: "idle", + lastFetchedAt: null, + }; + + const listeners: Set = new Set(); + let pollTimerId: ReturnType | null = null; + let sseSource: EventSource | null = null; + let sseReconnectTimerId: ReturnType | null = null; + let sseReconnectAttempts = 0; + let isFetching = false; + let isDestroyed = false; + let isStarted = false; + let refreshDebounceTimerId: ReturnType | null = null; + let refreshRequestedWhileFetching = false; + + function isDocumentHidden(): boolean { + return typeof document !== "undefined" && document.visibilityState === "hidden"; + } + + function shouldPoll(): boolean { + return isDocumentHidden() || sseSource === null; + } + + function pollIntervalMs(): number { + return isDocumentHidden() ? SLOW_POLL_INTERVAL_MS : POLL_INTERVAL_MS; + } + + function emitChange() { + for (const listener of listeners) { + listener(); + } + } + + function updateSnapshot(partial: Partial) { + const nextSnapshot = { ...snapshot, ...partial }; + if ( + nextSnapshot.workspace === snapshot.workspace + && nextSnapshot.fetchStatus === snapshot.fetchStatus + && nextSnapshot.lastFetchedAt === snapshot.lastFetchedAt + ) { + return; + } + snapshot = nextSnapshot; + emitChange(); + } + + async function fetchState() { + if (isDestroyed || !workspaceName) return; + if (isFetching) { + refreshRequestedWhileFetching = true; + return; + } + + isFetching = true; + refreshRequestedWhileFetching = false; + + if (snapshot.workspace === null) { + updateSnapshot({ fetchStatus: "fetching" }); + } + + try { + const response = await fetch( + `/api/v1/workspaces/${encodeURIComponent(workspaceName)}/state`, + ); + if (isDestroyed) return; + if (!response.ok) { + updateSnapshot({ fetchStatus: snapshot.workspace === null ? "error" : "idle" }); + return; + } + + const data = (await response.json()) as ApiWorkspaceEntry; + if (isDestroyed) return; + updateSnapshot({ + workspace: data, + fetchStatus: "idle", + lastFetchedAt: Date.now(), + }); + } catch { + if (!isDestroyed) { + updateSnapshot({ fetchStatus: snapshot.workspace === null ? "error" : "idle" }); + } + } finally { + isFetching = false; + + if (refreshRequestedWhileFetching && !isDestroyed) { + refreshRequestedWhileFetching = false; + void fetchState(); + } + } + } + + function syncPolling() { + if (pollTimerId !== null) { + clearTimeout(pollTimerId); + pollTimerId = null; + } + if (isDestroyed || !workspaceName || !shouldPoll()) return; + pollTimerId = setTimeout(() => { + pollTimerId = null; + void fetchState(); + syncPolling(); + }, pollIntervalMs()); + } + + function handleVisibilityChange() { + if (isDestroyed) return; + if (!isDocumentHidden()) { + void fetchState(); + if (sseSource === null && sseReconnectTimerId === null) { + connectSSESignal(); + } + } + syncPolling(); + } + + function sseBackoffDelay(): number { + return Math.min(SSE_BASE_BACKOFF_MS * Math.pow(2, sseReconnectAttempts), SSE_MAX_BACKOFF_MS); + } + + function scheduleSSEReconnect() { + if (isDestroyed) return; + if (sseReconnectTimerId !== null) { + clearTimeout(sseReconnectTimerId); + sseReconnectTimerId = null; + } + const delay = sseBackoffDelay(); + sseReconnectAttempts++; + sseReconnectTimerId = setTimeout(() => { + sseReconnectTimerId = null; + connectSSESignal(); + }, delay); + } + + function shouldRefreshForSignal(payload: SignalPayload): boolean { + if (!payload.workspace) { + return false; + } + return payload.workspace === workspaceName || payload.workspace === snapshot.workspace?.dir; + } + + function connectSSESignal() { + if (sseSource !== null || isDestroyed || !workspaceName || typeof EventSource === "undefined") return; + + sseSource = new EventSource("/api/v1/signal"); + + sseSource.onopen = () => { + sseReconnectAttempts = 0; + syncPolling(); + }; + + sseSource.onerror = () => { + if (sseSource !== null) { + sseSource.close(); + sseSource = null; + } + syncPolling(); + scheduleSSEReconnect(); + }; + + sseSource.addEventListener("reload", () => { + if (!isDestroyed && snapshot.workspace === null) { + void fetchState(); + } + }); + + sseSource.addEventListener("workspace", (event) => { + if (isDestroyed) { + return; + } + + try { + const payload = JSON.parse((event as MessageEvent).data) as SignalPayload; + if (shouldRefreshForSignal(payload)) { + void fetchState(); + } + } catch { + void fetchState(); + } + }); + } + + function start() { + if (isStarted || isDestroyed || !workspaceName) return; + isStarted = true; + + void fetchState(); + connectSSESignal(); + syncPolling(); + + if (typeof document !== "undefined") { + document.addEventListener("visibilitychange", handleVisibilityChange); + } + } + + function triggerRefresh() { + if (isDestroyed) return; + + if (refreshDebounceTimerId !== null) { + clearTimeout(refreshDebounceTimerId); + refreshDebounceTimerId = null; + } + + refreshDebounceTimerId = setTimeout(() => { + refreshDebounceTimerId = null; + void fetchState(); + syncPolling(); + }, DEBOUNCE_MS); + } + + function stop() { + if (isDestroyed) { + return; + } + + isDestroyed = true; + isStarted = false; + + if (pollTimerId !== null) { + clearTimeout(pollTimerId); + pollTimerId = null; + } + + if (refreshDebounceTimerId !== null) { + clearTimeout(refreshDebounceTimerId); + refreshDebounceTimerId = null; + } + + if (sseReconnectTimerId !== null) { + clearTimeout(sseReconnectTimerId); + sseReconnectTimerId = null; + } + + if (sseSource !== null) { + sseSource.close(); + sseSource = null; + } + + if (typeof document !== "undefined") { + document.removeEventListener("visibilitychange", handleVisibilityChange); + } + + listeners.clear(); + removeStore(); + } + + function subscribe(listener: Listener): () => void { + listeners.add(listener); + if (!isStarted && !isDestroyed) { + start(); + } + return () => { + listeners.delete(listener); + if (listeners.size === 0) { + stop(); + } + }; + } + + function getSnapshot(): WorkspacePageSnapshot { + return snapshot; + } + + function getServerSnapshot(): WorkspacePageSnapshot { + return { + workspace: null, + fetchStatus: "idle", + lastFetchedAt: null, + }; + } + + return { + subscribe, + getSnapshot, + getServerSnapshot, + stop, + triggerRefresh, + }; +} + +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); + if (existingStore) { + return existingStore; + } + + let createdStore!: WorkspacePageStore; + createdStore = createWorkspacePageStore(normalizedName, () => { + if (storeInstances.get(normalizedName) === createdStore) { + storeInstances.delete(normalizedName); + } + }); + storeInstances.set(normalizedName, createdStore); + return createdStore; +} + +export function resetWorkspacePageStateStores(): void { + for (const store of Array.from(storeInstances.values())) { + store.stop(); + } + storeInstances.clear(); +} + +export function triggerWorkspacePageRefresh(workspaceName: string): void { + const normalizedName = normalizeWorkspaceName(workspaceName); + const store = storeInstances.get(normalizedName); + if (!store) { + return; + } + store.triggerRefresh(); +} + +export function useWorkspacePageState(workspaceName: string): WorkspacePageSnapshot { + const store = getWorkspacePageStore(workspaceName); + return useSyncExternalStore( + store.subscribe, + store.getSnapshot, + store.getServerSnapshot, + ); +} diff --git a/cmd/sgai/webapp/src/pages/EditGoal.tsx b/cmd/sgai/webapp/src/pages/EditGoal.tsx index a1742b7..f213297 100644 --- a/cmd/sgai/webapp/src/pages/EditGoal.tsx +++ b/cmd/sgai/webapp/src/pages/EditGoal.tsx @@ -30,7 +30,7 @@ export function EditGoal(): JSX.Element { const titleRef = useRef(null); const goalRequestRef = useRef<{ target: string; - promise: ReturnType; + promise: ReturnType; } | null>(null); const matchingWorkspaces = workspaceSnapshot.workspaces.filter((candidate) => candidate.name === workspaceRouteName); const isAmbiguousWorkspaceRoute = matchingWorkspaces.length > 1; @@ -246,7 +246,7 @@ export function EditGoal(): JSX.Element {

      Edit GOAL.md

      - + import("./tabs/SessionTab").then((m) => ({ default: m.SessionTab }))); @@ -178,24 +175,11 @@ export function WorkspaceDetail(): JSX.Element | null { const [isResetDialogOpen, setIsResetDialogOpen] = useState(false); const [execTimeSeconds, setExecTimeSeconds] = useState(null); - const { workspaces, fetchStatus } = useFactoryState(); - const workspaceNameDisambiguators = useMemo(() => { - return buildWorkspaceNameDisambiguators(workspaces); - }, [workspaces]); - const matchingWorkspaceCount = useMemo(() => { - return workspaces.filter((workspace) => workspace.name === workspaceName).length; - }, [workspaceName, workspaces]); - - const detail = useMemo(() => { - return resolveWorkspaceByName(workspaces, workspaceName); - }, [workspaceName, workspaces]); + const { workspace: detail, fetchStatus } = useWorkspacePageState(workspaceName); const loading = fetchStatus === "fetching" && detail === null; - const isAmbiguousWorkspaceRoute = matchingWorkspaceCount > 1; const routeError = fetchStatus === "error" ? "Failed to load workspace state" - : isAmbiguousWorkspaceRoute - ? "Workspace route is ambiguous." - : "Workspace not found."; + : "Workspace not found."; useEffect(() => { if (previousWorkspaceRef.current !== workspaceRouteKey) { @@ -240,25 +224,6 @@ export function WorkspaceDetail(): JSX.Element | null { const redirectTab = resolveRedirectTab({ requestedTab, isForkedRoot, showForkTab }); const activeTab = redirectTab ?? requestedTab; - const [actionOutputOpen, setActionOutputOpen] = useState(false); - const { - output: actionOutput, - isRunning: isActionRunning, - runError: actionRunError, - startActionRun, - stopRun: stopActionRun, - outputRef: actionOutputRef, - } = useAdhocRun({ workspaceName, skipModelsFetch: true }); - - const handleActionClick = useCallback(( - action: ApiActionEntry, - variables: Record, - targetWorkspaceName = workspaceName, - ) => { - setActionOutputOpen(true); - void startActionRun(action.name, variables, targetWorkspaceName); - }, [startActionRun, workspaceName]); - useEffect(() => { if (!detail || !redirectTab) return; navigate(buildWorkspacePath(detail, redirectTab), { replace: true }); @@ -270,15 +235,11 @@ export function WorkspaceDetail(): JSX.Element | null { return ; } - const detailLabel = getWorkspaceDisplayLabel(detail, workspaceNameDisambiguators); + const detailLabel = getWorkspaceBaseLabel(detail); if (!detail.hasSgai && !detail.isRoot) { return ; } - const parentRoot = detail.isFork - ? workspaces.find((workspace) => workspace.forks?.some((fork) => isSameWorkspace(fork, detail))) - : undefined; - const effectiveRunning = runningOverride !== null ? runningOverride : (detail.running ?? false); const totalExecTime = detail.totalExecTime?.trim() ?? ""; @@ -308,7 +269,6 @@ export function WorkspaceDetail(): JSX.Element | null { void (async () => { try { const result = await api.workspaces.start(workspaceName, false); - triggerFactoryRefresh(); if (result.running) { setRunningOverride(true); } @@ -327,7 +287,6 @@ export function WorkspaceDetail(): JSX.Element | null { void (async () => { try { const result = await api.workspaces.stop(workspaceName); - triggerFactoryRefresh(); if (!result.running) { setRunningOverride(false); } @@ -346,7 +305,6 @@ export function WorkspaceDetail(): JSX.Element | null { void (async () => { try { await api.workspaces.start(workspaceName, true); - triggerFactoryRefresh(); setRunningOverride(true); } catch (err) { setActionError(err instanceof Error ? err.message : "Failed to start self-drive session"); @@ -363,7 +321,6 @@ export function WorkspaceDetail(): JSX.Element | null { void (async () => { try { await api.workspaces.togglePin(workspaceName); - triggerFactoryRefresh(); } catch (err) { setActionError(err instanceof Error ? err.message : "Failed to toggle pin"); } finally { @@ -388,10 +345,6 @@ export function WorkspaceDetail(): JSX.Element | null { }; const handleRepositoryActionCompleted = () => { - if (detail.isFork && parentRoot) { - navigate(buildWorkspacePath(parentRoot, "forks")); - return; - } navigate("/"); }; @@ -409,7 +362,6 @@ export function WorkspaceDetail(): JSX.Element | null { void (async () => { try { await api.workspaces.reset(workspaceName); - triggerFactoryRefresh(); setIsResetDialogOpen(false); } catch (err) { setActionError(err instanceof Error ? err.message : "Failed to reset workspace"); @@ -701,60 +653,105 @@ export function WorkspaceDetail(): JSX.Element | null {
    - {isForkedRoot && (actionRunError || isActionRunning || actionOutput) ? ( -
    - {actionRunError ? ( -

    {actionRunError}

    - ) : null} - {(isActionRunning || actionOutput) ? ( -
    setActionOutputOpen((e.target as HTMLDetailsElement).open)}> - - - 
    +              {({ onActionClick, isActionRunning }) => (
+                }>
+                  
+                
+              )}
+            
+          ) : (
+            }>
+              
+            
+          )}
+
    +
    + ); +} + +function ForkRootActionOutputSlot({ + workspaceName, + children, +}: { + workspaceName: string; + children: (controls: { + onActionClick: (action: ApiActionEntry, variables: Record, targetWorkspaceName: string) => void; + isActionRunning: boolean; + }) => ReactNode; +}) { + const [actionOutputOpen, setActionOutputOpen] = useState(false); + const { + output: actionOutput, + isRunning: isActionRunning, + runError: actionRunError, + startActionRun, + stopRun: stopActionRun, + outputRef: actionOutputRef, + } = useAdhocRun({ workspaceName, skipModelsFetch: true }); + + const handleActionClick = useCallback(( + action: ApiActionEntry, + variables: Record, + targetWorkspaceName = workspaceName, + ) => { + setActionOutputOpen(true); + void startActionRun(action.name, variables, targetWorkspaceName); + }, [startActionRun, workspaceName]); + + return ( + <> + {(actionRunError || isActionRunning || actionOutput) ? ( +
    + {actionRunError ? ( +

    {actionRunError}

    + ) : null} + {(isActionRunning || actionOutput) ? ( +
    setActionOutputOpen((e.target as HTMLDetailsElement).open)}> + +
    - ) : null} -
    + + Stop + + )} + + 
    +                {actionOutput || (isActionRunning ? "Running..." : "")}
+
    + ) : null} - }> - -
-
+ ) : null} + {children({ onActionClick: handleActionClick, isActionRunning })} + ); } @@ -769,25 +766,13 @@ function TabSkeleton() { function TabContent({ activeTab, - workspaceName, - currentModel, - goalContent, - pmContent, - hasProjectMgmt, - actions, - actionConfigError, + detail, onActionClick, isActionRunning, showForkTab, }: { activeTab: string; - workspaceName: string; - currentModel?: string; - goalContent?: string; - pmContent?: string; - hasProjectMgmt?: boolean; - actions?: ApiActionEntry[]; - actionConfigError?: string; + detail: ApiWorkspaceEntry; onActionClick?: (action: ApiActionEntry, variables: Record, targetWorkspaceName: string) => void; isActionRunning?: boolean; showForkTab: boolean; @@ -796,29 +781,47 @@ function TabContent({ case "progress": return ( ); case "fork": - return showForkTab ? : ; + return showForkTab ? : ; case "log": - return ; + return ; case "messages": - return ; + return ; case "internals": - return ; + return ( + + ); case "run": - return ; + return ; case "forks": return ( diff --git a/cmd/sgai/webapp/src/pages/__tests__/EditGoal.test.tsx b/cmd/sgai/webapp/src/pages/__tests__/EditGoal.test.tsx index aa31fa6..057682b 100644 --- a/cmd/sgai/webapp/src/pages/__tests__/EditGoal.test.tsx +++ b/cmd/sgai/webapp/src/pages/__tests__/EditGoal.test.tsx @@ -6,6 +6,7 @@ import { MemoryRouter, Routes, Route, RouterProvider, createMemoryRouter, useNav import { TooltipProvider } from "@/components/ui/tooltip"; import { SidebarProvider } from "@/components/ui/sidebar"; import * as factoryStateModule from "@/lib/factory-state"; +import * as workspacePageStateModule from "@/lib/workspace-page-state"; import { api } from "@/lib/api"; import * as markdownEditorModule from "@/components/MarkdownEditor"; import * as useAdhocRunModule from "@/hooks/useAdhocRun"; @@ -63,43 +64,43 @@ const mockUpdateGoal = mock(() => Promise.resolve({ updated: true, workspace: "t const mockTriggerFactoryRefresh = mock(() => {}); const mockMarkdownEditor = ({ - value, - onChange, - disabled, - onSubmitShortcut, - }: { - value: string; - onChange: (v: string) => void; - disabled: boolean; - onSubmitShortcut?: () => void; - }) => ( -
-