From d82bf20577b2b686a4bac4a1263d816892dc45de Mon Sep 17 00:00:00 2001 From: igor Date: Fri, 29 May 2026 19:33:05 -0300 Subject: [PATCH 1/5] fix: safe workspace delete and session remove command Workspace delete now guards against active sessions: wraps the operation in a transaction, checks for active sessions using a new listActiveSessionIDsByWorkspace helper, cleans up stopped sessions before removing the workspace row, and returns ErrWorkspaceHasActiveSessions (HTTP 409) when live sessions block the delete. Adds agh session remove CLI command that deletes a session and its persisted history via the daemon UDS client. - globaldb: transactional DeleteWorkspace with active-session guard - workspace: export ErrWorkspaceHasActiveSessions sentinel error - api/core: map ErrWorkspaceHasActiveSessions to HTTP 409 - cli/client: add DeleteSession to DaemonClient interface and implement on unixSocketClient - cli/session: add session remove subcommand --- internal/api/core/session_workspace.go | 3 +- .../core/session_workspace_internal_test.go | 3 + internal/cli/client.go | 16 +++++ internal/cli/helpers_test.go | 8 +++ internal/cli/session.go | 28 ++++++++ internal/cli/session_test.go | 40 ++++++++++++ internal/store/globaldb/global_db_test.go | 65 ++++++++++++++++--- .../store/globaldb/global_db_workspace.go | 65 ++++++++++++++++++- internal/workspace/workspace.go | 2 + internal/workspace/workspace_test.go | 6 ++ 10 files changed, 225 insertions(+), 11 deletions(-) diff --git a/internal/api/core/session_workspace.go b/internal/api/core/session_workspace.go index 8f159ed2e..3b9031cbf 100644 --- a/internal/api/core/session_workspace.go +++ b/internal/api/core/session_workspace.go @@ -137,7 +137,8 @@ func statusForWorkspaceError(err error) int { return http.StatusGone case errors.Is(err, workspacepkg.ErrWorkspaceNameTaken), errors.Is(err, workspacepkg.ErrWorkspacePathTaken), - errors.Is(err, workspacepkg.ErrWorkspaceHasSessions): + errors.Is(err, workspacepkg.ErrWorkspaceHasSessions), + errors.Is(err, workspacepkg.ErrWorkspaceHasActiveSessions): return http.StatusConflict case errors.Is(err, workspacepkg.ErrWorkspaceResolverUnavailable): return http.StatusServiceUnavailable diff --git a/internal/api/core/session_workspace_internal_test.go b/internal/api/core/session_workspace_internal_test.go index 035b4b53c..8259cb98f 100644 --- a/internal/api/core/session_workspace_internal_test.go +++ b/internal/api/core/session_workspace_internal_test.go @@ -149,6 +149,9 @@ func TestSessionWorkspaceStatusMappings(t *testing.T) { if got := statusForWorkspaceError(workspacepkg.ErrWorkspaceHasSessions); got != http.StatusConflict { t.Fatalf("statusForWorkspaceError(has sessions) = %d, want %d", got, http.StatusConflict) } + if got := statusForWorkspaceError(workspacepkg.ErrWorkspaceHasActiveSessions); got != http.StatusConflict { + t.Fatalf("statusForWorkspaceError(has active sessions) = %d, want %d", got, http.StatusConflict) + } if got := statusForWorkspaceError( workspacepkg.ErrWorkspaceResolverUnavailable, ); got != http.StatusServiceUnavailable { diff --git a/internal/cli/client.go b/internal/cli/client.go index eb32e081f..74dfda449 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -178,6 +178,7 @@ type DaemonClient interface { InspectSession(ctx context.Context, id string, query SessionInspectQuery) (SessionInspectRecord, error) RefreshSessionSoul(ctx context.Context, id string, request SessionSoulRefreshRequest) (AgentSoulRecord, error) StopSession(ctx context.Context, id string) error + DeleteSession(ctx context.Context, id string) error ResumeSession(ctx context.Context, id string) (SessionRecord, error) SessionRecap(ctx context.Context, id string, limit int) (SessionRecapRecord, error) RepairSession(ctx context.Context, id string, query SessionRepairQuery) (SessionRepairRecord, error) @@ -2622,6 +2623,21 @@ func (c *unixSocketClient) StopSession(ctx context.Context, id string) error { ) } +func (c *unixSocketClient) DeleteSession(ctx context.Context, id string) error { + path, err := c.sessionScopedPath(ctx, id, "") + if err != nil { + return err + } + return c.doJSON( + ctx, + http.MethodDelete, + path, + nil, + nil, + nil, + ) +} + func (c *unixSocketClient) ResumeSession(ctx context.Context, id string) (SessionRecord, error) { var response struct { Session SessionRecord `json:"session"` diff --git a/internal/cli/helpers_test.go b/internal/cli/helpers_test.go index 893bc4dee..d2312e7ab 100644 --- a/internal/cli/helpers_test.go +++ b/internal/cli/helpers_test.go @@ -101,6 +101,7 @@ type stubClient struct { inspectSessionFn func(context.Context, string, SessionInspectQuery) (SessionInspectRecord, error) refreshSessionSoulFn func(context.Context, string, SessionSoulRefreshRequest) (AgentSoulRecord, error) stopSessionFn func(context.Context, string) error + deleteSessionFn func(context.Context, string) error resumeSessionFn func(context.Context, string) (SessionRecord, error) sessionRecapFn func(context.Context, string, int) (SessionRecapRecord, error) repairSessionFn func(context.Context, string, SessionRepairQuery) (SessionRepairRecord, error) @@ -1030,6 +1031,13 @@ func (s *stubClient) StopSession(ctx context.Context, id string) error { return errors.New("unexpected StopSession call") } +func (s *stubClient) DeleteSession(ctx context.Context, id string) error { + if s.deleteSessionFn != nil { + return s.deleteSessionFn(ctx, id) + } + return errors.New("unexpected DeleteSession call") +} + func (s *stubClient) ResumeSession(ctx context.Context, id string) (SessionRecord, error) { if s.resumeSessionFn != nil { return s.resumeSessionFn(ctx, id) diff --git a/internal/cli/session.go b/internal/cli/session.go index 3cfcf20bd..7d7d6215e 100644 --- a/internal/cli/session.go +++ b/internal/cli/session.go @@ -62,6 +62,7 @@ func newSessionCommand(deps commandDeps) *cobra.Command { cmd.AddCommand(newSessionCreateCommand(deps)) cmd.AddCommand(newSessionListCommand(deps)) cmd.AddCommand(newSessionStopCommand(deps)) + cmd.AddCommand(newSessionRemoveCommand(deps)) cmd.AddCommand(newSessionSoulCommand(deps)) cmd.AddCommand(newSessionHealthCommand(deps)) cmd.AddCommand(newSessionStatusCommand(deps)) @@ -225,6 +226,33 @@ func newSessionStopCommand(deps commandDeps) *cobra.Command { } } +func newSessionRemoveCommand(deps commandDeps) *cobra.Command { + return &cobra.Command{ + Use: "remove ", + Short: "Remove a session and its persisted history", + Example: ` # Remove a stopped session + agh session remove sess_1234 + + # Remove an active session (stops it first) + agh session remove sess_1234`, + Args: exactOneNonBlankArg(), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := clientFromDeps(deps) + if err != nil { + return err + } + info, err := client.GetSession(cmd.Context(), args[0]) + if err != nil { + return err + } + if err := client.DeleteSession(cmd.Context(), args[0]); err != nil { + return err + } + return writeCommandOutput(cmd, sessionBundle(info, deps.now)) + }, + } +} + func newSessionStatusCommand(deps commandDeps) *cobra.Command { return &cobra.Command{ Use: "status ", diff --git a/internal/cli/session_test.go b/internal/cli/session_test.go index 3f055f046..baff14e7c 100644 --- a/internal/cli/session_test.go +++ b/internal/cli/session_test.go @@ -613,6 +613,46 @@ func TestSessionStopFetchesUpdatedSession(t *testing.T) { } } +func TestSessionRemoveDeletesSession(t *testing.T) { + t.Parallel() + + var deletedID string + + deps := newTestDeps(t, &stubClient{ + getSessionFn: func(_ context.Context, id string) (SessionRecord, error) { + return SessionRecord{ + ID: id, + AgentName: "coder", + WorkspaceID: "ws-1", + WorkspacePath: "/workspace/project", + State: session.StateStopped, + CreatedAt: fixedTestNow, + UpdatedAt: fixedTestNow, + }, nil + }, + deleteSessionFn: func(_ context.Context, id string) error { + deletedID = id + return nil + }, + }) + + stdout, _, err := executeRootCommand(t, deps, "session", "remove", "sess-1", "-o", "json") + if err != nil { + t.Fatalf("executeRootCommand() error = %v", err) + } + if deletedID != "sess-1" { + t.Fatalf("DeleteSession() id = %q, want %q", deletedID, "sess-1") + } + + var decoded SessionRecord + if err := json.Unmarshal([]byte(stdout), &decoded); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if decoded.ID != "sess-1" { + t.Fatalf("decoded.ID = %q, want %q", decoded.ID, "sess-1") + } +} + func TestSessionStatusReturnsHealthStatus(t *testing.T) { t.Parallel() diff --git a/internal/store/globaldb/global_db_test.go b/internal/store/globaldb/global_db_test.go index 4c6c9f57e..81460371e 100644 --- a/internal/store/globaldb/global_db_test.go +++ b/internal/store/globaldb/global_db_test.go @@ -1549,7 +1549,7 @@ func TestGlobalDBWorkspaceCRUDAndLookups(t *testing.T) { } } -func TestGlobalDBDeleteWorkspaceReturnsHasSessionsWhenReferenced(t *testing.T) { +func TestGlobalDBDeleteWorkspaceCascadeDeletesStoppedSessions(t *testing.T) { t.Parallel() globalDB := openTestGlobalDB(t) @@ -1563,6 +1563,58 @@ func TestGlobalDBDeleteWorkspaceReturnsHasSessionsWhenReferenced(t *testing.T) { ID: "sess-delete-guard", AgentName: "coder", WorkspaceID: workspaceID, + State: "stopped", + CreatedAt: time.Date(2026, 4, 3, 14, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2026, 4, 3, 14, 0, 0, 0, time.UTC), + }); err != nil { + t.Fatalf("RegisterSession() error = %v", err) + } + + if err := globalDB.DeleteWorkspace(testutil.Context(t), workspaceID); err != nil { + t.Fatalf("DeleteWorkspace() error = %v, want nil", err) + } + + sessions, err := globalDB.ListSessions(testutil.Context(t), SessionListQuery{ + WorkspaceID: workspaceID, + }) + if err != nil { + t.Fatalf("ListSessions() error = %v", err) + } + if len(sessions) != 0 { + t.Fatalf("ListSessions() = %d sessions, want 0 (cascade delete)", len(sessions)) + } +} + +func TestGlobalDBDeleteWorkspaceWithoutSessions(t *testing.T) { + t.Parallel() + + globalDB := openTestGlobalDB(t) + workspaceID := registerWorkspaceForGlobalTests( + t, + globalDB, + "ws-no-sessions", + filepath.Join(t.TempDir(), "ws-no-sessions"), + ) + + if err := globalDB.DeleteWorkspace(testutil.Context(t), workspaceID); err != nil { + t.Fatalf("DeleteWorkspace() error = %v, want nil", err) + } +} + +func TestGlobalDBDeleteWorkspaceRejectsActiveSessions(t *testing.T) { + t.Parallel() + + globalDB := openTestGlobalDB(t) + workspaceID := registerWorkspaceForGlobalTests( + t, + globalDB, + "ws-active-sessions", + filepath.Join(t.TempDir(), "ws-active-sessions"), + ) + if err := globalDB.RegisterSession(testutil.Context(t), SessionInfo{ + ID: "sess-active-guard", + AgentName: "coder", + WorkspaceID: workspaceID, State: "active", CreatedAt: time.Date(2026, 4, 3, 14, 0, 0, 0, time.UTC), UpdatedAt: time.Date(2026, 4, 3, 14, 0, 0, 0, time.UTC), @@ -1570,14 +1622,9 @@ func TestGlobalDBDeleteWorkspaceReturnsHasSessionsWhenReferenced(t *testing.T) { t.Fatalf("RegisterSession() error = %v", err) } - if err := globalDB.DeleteWorkspace( - testutil.Context(t), - workspaceID, - ); !errors.Is( - err, - aghworkspace.ErrWorkspaceHasSessions, - ) { - t.Fatalf("DeleteWorkspace() error = %v, want ErrWorkspaceHasSessions", err) + err := globalDB.DeleteWorkspace(testutil.Context(t), workspaceID) + if !errors.Is(err, aghworkspace.ErrWorkspaceHasActiveSessions) { + t.Fatalf("DeleteWorkspace() error = %v, want ErrWorkspaceHasActiveSessions", err) } } diff --git a/internal/store/globaldb/global_db_workspace.go b/internal/store/globaldb/global_db_workspace.go index b6626a598..164886675 100644 --- a/internal/store/globaldb/global_db_workspace.go +++ b/internal/store/globaldb/global_db_workspace.go @@ -83,6 +83,8 @@ func (g *GlobalDB) UpdateWorkspace(ctx context.Context, ws aghworkspace.Workspac } // DeleteWorkspace removes a persisted workspace registration row. +// It refuses to delete if any active sessions reference the workspace. +// Stopped or orphaned sessions are cleaned up automatically before deletion. func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { if err := g.checkReady(ctx, "delete workspace"); err != nil { return err @@ -93,7 +95,32 @@ func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { return errors.New("store: workspace id is required") } - result, err := g.db.ExecContext(ctx, `DELETE FROM workspaces WHERE id = ?`, trimmedID) + tx, err := g.db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("store: begin delete workspace transaction %q: %w", trimmedID, err) + } + defer func() { + _ = tx.Rollback() + }() + + activeSessions, err := g.listActiveSessionIDsByWorkspace(ctx, tx, trimmedID) + if err != nil { + return err + } + if len(activeSessions) > 0 { + return fmt.Errorf( + "store: delete workspace %q: %w: %s", + trimmedID, + aghworkspace.ErrWorkspaceHasActiveSessions, + strings.Join(activeSessions, ", "), + ) + } + + if _, err := tx.ExecContext(ctx, `DELETE FROM sessions WHERE workspace_id = ?`, trimmedID); err != nil { + return fmt.Errorf("store: delete stopped sessions for workspace %q: %w", trimmedID, err) + } + + result, err := tx.ExecContext(ctx, `DELETE FROM workspaces WHERE id = ?`, trimmedID) if err != nil { return fmt.Errorf("store: delete workspace %q: %w", trimmedID, mapWorkspaceConstraintError(err)) } @@ -106,9 +133,45 @@ func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { return fmt.Errorf("store: workspace %q: %w", trimmedID, aghworkspace.ErrWorkspaceNotFound) } + if err := tx.Commit(); err != nil { + return fmt.Errorf("store: commit delete workspace %q: %w", trimmedID, err) + } + return nil } +func (g *GlobalDB) listActiveSessionIDsByWorkspace( + ctx context.Context, + tx *sql.Tx, + workspaceID string, +) ([]string, error) { + rows, err := tx.QueryContext( + ctx, + `SELECT id FROM sessions WHERE workspace_id = ? AND state = 'active'`, + workspaceID, + ) + if err != nil { + return nil, fmt.Errorf("store: list active sessions for workspace %q: %w", workspaceID, err) + } + defer func() { + _ = rows.Close() + }() + + var ids []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, fmt.Errorf("store: scan active session id for workspace %q: %w", workspaceID, err) + } + ids = append(ids, id) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("store: iterate active sessions for workspace %q: %w", workspaceID, err) + } + + return ids, nil +} + // GetWorkspace loads a workspace registration by primary key. func (g *GlobalDB) GetWorkspace(ctx context.Context, id string) (aghworkspace.Workspace, error) { if err := g.checkReady(ctx, "get workspace"); err != nil { diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go index b4a7357f7..eabab4c8e 100644 --- a/internal/workspace/workspace.go +++ b/internal/workspace/workspace.go @@ -26,6 +26,8 @@ var ( ErrWorkspacePathTaken = errors.New("workspace path already registered") // ErrWorkspaceHasSessions reports that a workspace cannot be deleted because sessions still reference it. ErrWorkspaceHasSessions = errors.New("workspace has sessions") + // ErrWorkspaceHasActiveSessions reports that a workspace cannot be deleted because active sessions are running. + ErrWorkspaceHasActiveSessions = errors.New("workspace has active sessions") // ErrWorkspaceIdentityInvalid reports a malformed .agh/workspace.toml identity file. ErrWorkspaceIdentityInvalid = errors.New("workspace identity invalid") // ErrWorkspaceIdentityPermissionDenied reports a fail-closed identity file permission failure. diff --git a/internal/workspace/workspace_test.go b/internal/workspace/workspace_test.go index e1e0c703b..0709e9272 100644 --- a/internal/workspace/workspace_test.go +++ b/internal/workspace/workspace_test.go @@ -25,6 +25,7 @@ func TestWorkspaceErrorsMatchViaErrorsIs(t *testing.T) { {name: "name taken", sentinel: workspace.ErrWorkspaceNameTaken}, {name: "path taken", sentinel: workspace.ErrWorkspacePathTaken}, {name: "has sessions", sentinel: workspace.ErrWorkspaceHasSessions}, + {name: "has active sessions", sentinel: workspace.ErrWorkspaceHasActiveSessions}, } for _, tt := range tests { @@ -67,6 +68,11 @@ func TestWorkspaceErrorsAreDistinct(t *testing.T) { left: workspace.ErrWorkspacePathTaken, want: workspace.ErrWorkspaceHasSessions, }, + { + name: "has sessions does not match has active sessions", + left: workspace.ErrWorkspaceHasSessions, + want: workspace.ErrWorkspaceHasActiveSessions, + }, } for _, tt := range tests { From f86bb5716f3603dd7365c67f99ed68d2237f3c38 Mon Sep 17 00:00:00 2001 From: igor Date: Fri, 29 May 2026 20:00:18 -0300 Subject: [PATCH 2/5] feat: add agh open command to launch web ui in browser Adds `agh open` CLI command that fetches the daemon's HTTP address via DaemonStatus and opens the web UI in the default system browser. Also fixes errcheck lint violation in DeleteWorkspace deferred rollback. --- internal/cli/open.go | 50 +++++++++++++++++++ internal/cli/root.go | 1 + .../store/globaldb/global_db_workspace.go | 6 ++- 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 internal/cli/open.go diff --git a/internal/cli/open.go b/internal/cli/open.go new file mode 100644 index 000000000..b9f4582d0 --- /dev/null +++ b/internal/cli/open.go @@ -0,0 +1,50 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "os/exec" + "runtime" + + "github.com/spf13/cobra" +) + +func newOpenCommand(deps commandDeps) *cobra.Command { + return &cobra.Command{ + Use: "open", + Short: "Open the AGH web UI in the default browser", + RunE: func(cmd *cobra.Command, _ []string) error { + ctx := cmd.Context() + + client, err := clientFromDeps(deps) + if err != nil { + return err + } + + status, err := client.DaemonStatus(ctx) + if err != nil { + return fmt.Errorf("open: daemon is not running: %w", err) + } + if status.HTTPHost == "" || status.HTTPPort == 0 { + return errors.New("open: daemon did not report a valid HTTP address") + } + + url := fmt.Sprintf("http://%s:%d", status.HTTPHost, status.HTTPPort) + return openBrowser(ctx, url) + }, + } +} + +func openBrowser(ctx context.Context, url string) error { + var cmd *exec.Cmd + switch runtime.GOOS { + case "darwin": + cmd = exec.CommandContext(ctx, "open", url) + case "windows": + cmd = exec.CommandContext(ctx, "cmd", "/c", "start", url) + default: + cmd = exec.CommandContext(ctx, "xdg-open", url) + } + return cmd.Start() +} diff --git a/internal/cli/root.go b/internal/cli/root.go index dc907517e..3df0463b6 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -142,6 +142,7 @@ func newRootCommand(deps commandDeps) *cobra.Command { cmd.AddCommand(newMCPCommand(deps)) cmd.AddCommand(newLogsCommand(deps)) cmd.AddCommand(newWhoamiCommand(deps)) + cmd.AddCommand(newOpenCommand(deps)) cmd.AddCommand(newDocCommand()) return cmd diff --git a/internal/store/globaldb/global_db_workspace.go b/internal/store/globaldb/global_db_workspace.go index 164886675..cb798eab0 100644 --- a/internal/store/globaldb/global_db_workspace.go +++ b/internal/store/globaldb/global_db_workspace.go @@ -85,7 +85,7 @@ func (g *GlobalDB) UpdateWorkspace(ctx context.Context, ws aghworkspace.Workspac // DeleteWorkspace removes a persisted workspace registration row. // It refuses to delete if any active sessions reference the workspace. // Stopped or orphaned sessions are cleaned up automatically before deletion. -func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { +func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) (retErr error) { if err := g.checkReady(ctx, "delete workspace"); err != nil { return err } @@ -100,7 +100,9 @@ func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { return fmt.Errorf("store: begin delete workspace transaction %q: %w", trimmedID, err) } defer func() { - _ = tx.Rollback() + if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { + retErr = rollbackErr + } }() activeSessions, err := g.listActiveSessionIDsByWorkspace(ctx, tx, trimmedID) From 9f86427b4330557849dc6ee68eb92ccf959e9067 Mon Sep 17 00:00:00 2001 From: igor Date: Fri, 29 May 2026 20:06:58 -0300 Subject: [PATCH 3/5] fix: use ExecuteWrite for workspace delete and fix test naming convention - Replace BeginTx with store.ExecuteWrite (BEGIN IMMEDIATE with SQLITE_BUSY retry) in DeleteWorkspace for correct write lock acquisition - Add justification comment for rows.Close() blank assignment in listActiveSessionIDsByWorkspace - Wrap TestSessionRemoveDeletesSession body in t.Run("Should ...") subtest per project naming convention --- internal/cli/session_test.go | 68 ++++++++-------- .../store/globaldb/global_db_workspace.go | 79 ++++++++----------- 2 files changed, 70 insertions(+), 77 deletions(-) diff --git a/internal/cli/session_test.go b/internal/cli/session_test.go index baff14e7c..48677fcd8 100644 --- a/internal/cli/session_test.go +++ b/internal/cli/session_test.go @@ -616,41 +616,45 @@ func TestSessionStopFetchesUpdatedSession(t *testing.T) { func TestSessionRemoveDeletesSession(t *testing.T) { t.Parallel() - var deletedID string + t.Run("Should delete session and return session record", func(t *testing.T) { + t.Parallel() - deps := newTestDeps(t, &stubClient{ - getSessionFn: func(_ context.Context, id string) (SessionRecord, error) { - return SessionRecord{ - ID: id, - AgentName: "coder", - WorkspaceID: "ws-1", - WorkspacePath: "/workspace/project", - State: session.StateStopped, - CreatedAt: fixedTestNow, - UpdatedAt: fixedTestNow, - }, nil - }, - deleteSessionFn: func(_ context.Context, id string) error { - deletedID = id - return nil - }, - }) + var deletedID string - stdout, _, err := executeRootCommand(t, deps, "session", "remove", "sess-1", "-o", "json") - if err != nil { - t.Fatalf("executeRootCommand() error = %v", err) - } - if deletedID != "sess-1" { - t.Fatalf("DeleteSession() id = %q, want %q", deletedID, "sess-1") - } + deps := newTestDeps(t, &stubClient{ + getSessionFn: func(_ context.Context, id string) (SessionRecord, error) { + return SessionRecord{ + ID: id, + AgentName: "coder", + WorkspaceID: "ws-1", + WorkspacePath: "/workspace/project", + State: session.StateStopped, + CreatedAt: fixedTestNow, + UpdatedAt: fixedTestNow, + }, nil + }, + deleteSessionFn: func(_ context.Context, id string) error { + deletedID = id + return nil + }, + }) - var decoded SessionRecord - if err := json.Unmarshal([]byte(stdout), &decoded); err != nil { - t.Fatalf("json.Unmarshal() error = %v", err) - } - if decoded.ID != "sess-1" { - t.Fatalf("decoded.ID = %q, want %q", decoded.ID, "sess-1") - } + stdout, _, err := executeRootCommand(t, deps, "session", "remove", "sess-1", "-o", "json") + if err != nil { + t.Fatalf("executeRootCommand() error = %v", err) + } + if deletedID != "sess-1" { + t.Fatalf("DeleteSession() id = %q, want %q", deletedID, "sess-1") + } + + var decoded SessionRecord + if err := json.Unmarshal([]byte(stdout), &decoded); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if decoded.ID != "sess-1" { + t.Fatalf("decoded.ID = %q, want %q", decoded.ID, "sess-1") + } + }) } func TestSessionStatusReturnsHealthStatus(t *testing.T) { diff --git a/internal/store/globaldb/global_db_workspace.go b/internal/store/globaldb/global_db_workspace.go index cb798eab0..145501e15 100644 --- a/internal/store/globaldb/global_db_workspace.go +++ b/internal/store/globaldb/global_db_workspace.go @@ -85,7 +85,7 @@ func (g *GlobalDB) UpdateWorkspace(ctx context.Context, ws aghworkspace.Workspac // DeleteWorkspace removes a persisted workspace registration row. // It refuses to delete if any active sessions reference the workspace. // Stopped or orphaned sessions are cleaned up automatically before deletion. -func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) (retErr error) { +func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) error { if err := g.checkReady(ctx, "delete workspace"); err != nil { return err } @@ -95,56 +95,44 @@ func (g *GlobalDB) DeleteWorkspace(ctx context.Context, id string) (retErr error return errors.New("store: workspace id is required") } - tx, err := g.db.BeginTx(ctx, nil) - if err != nil { - return fmt.Errorf("store: begin delete workspace transaction %q: %w", trimmedID, err) - } - defer func() { - if rollbackErr := tx.Rollback(); rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { - retErr = rollbackErr + return store.ExecuteWrite(ctx, g.db, func(ctx context.Context, tx *store.WriteTx) error { + activeSessions, err := g.listActiveSessionIDsByWorkspace(ctx, tx, trimmedID) + if err != nil { + return err + } + if len(activeSessions) > 0 { + return fmt.Errorf( + "store: delete workspace %q: %w: %s", + trimmedID, + aghworkspace.ErrWorkspaceHasActiveSessions, + strings.Join(activeSessions, ", "), + ) } - }() - - activeSessions, err := g.listActiveSessionIDsByWorkspace(ctx, tx, trimmedID) - if err != nil { - return err - } - if len(activeSessions) > 0 { - return fmt.Errorf( - "store: delete workspace %q: %w: %s", - trimmedID, - aghworkspace.ErrWorkspaceHasActiveSessions, - strings.Join(activeSessions, ", "), - ) - } - - if _, err := tx.ExecContext(ctx, `DELETE FROM sessions WHERE workspace_id = ?`, trimmedID); err != nil { - return fmt.Errorf("store: delete stopped sessions for workspace %q: %w", trimmedID, err) - } - result, err := tx.ExecContext(ctx, `DELETE FROM workspaces WHERE id = ?`, trimmedID) - if err != nil { - return fmt.Errorf("store: delete workspace %q: %w", trimmedID, mapWorkspaceConstraintError(err)) - } + if _, err := tx.ExecContext(ctx, `DELETE FROM sessions WHERE workspace_id = ?`, trimmedID); err != nil { + return fmt.Errorf("store: delete stopped sessions for workspace %q: %w", trimmedID, err) + } - affected, err := result.RowsAffected() - if err != nil { - return fmt.Errorf("store: rows affected for workspace %q: %w", trimmedID, err) - } - if affected == 0 { - return fmt.Errorf("store: workspace %q: %w", trimmedID, aghworkspace.ErrWorkspaceNotFound) - } + result, err := tx.ExecContext(ctx, `DELETE FROM workspaces WHERE id = ?`, trimmedID) + if err != nil { + return fmt.Errorf("store: delete workspace %q: %w", trimmedID, mapWorkspaceConstraintError(err)) + } - if err := tx.Commit(); err != nil { - return fmt.Errorf("store: commit delete workspace %q: %w", trimmedID, err) - } + affected, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("store: rows affected for workspace %q: %w", trimmedID, err) + } + if affected == 0 { + return fmt.Errorf("store: workspace %q: %w", trimmedID, aghworkspace.ErrWorkspaceNotFound) + } - return nil + return nil + }) } func (g *GlobalDB) listActiveSessionIDsByWorkspace( ctx context.Context, - tx *sql.Tx, + tx *store.WriteTx, workspaceID string, ) ([]string, error) { rows, err := tx.QueryContext( @@ -155,9 +143,10 @@ func (g *GlobalDB) listActiveSessionIDsByWorkspace( if err != nil { return nil, fmt.Errorf("store: list active sessions for workspace %q: %w", workspaceID, err) } - defer func() { - _ = rows.Close() - }() + // rows.Close error is not actionable here: any real failure is already + // captured by rows.Err() below, and the caller cannot recover from a + // close-only error on a read-only result set. + defer func() { _ = rows.Close() }() var ids []string for rows.Next() { From cd37d18b1aa2cb156e47bd64f15a420b76007701 Mon Sep 17 00:00:00 2001 From: Pedro Nauck Date: Tue, 2 Jun 2026 11:30:46 -0300 Subject: [PATCH 4/5] fix: clean workspace session artifacts --- internal/api/core/memory_workspace_test.go | 1 + internal/api/core/workspaces.go | 53 ++++++++ internal/api/httpapi/handlers_error_test.go | 45 +++++++ internal/api/httpapi/handlers_test.go | 57 +++++++++ internal/cli/cli_integration_test.go | 114 ++++++++++++++++++ internal/cli/session.go | 3 - internal/cli/workspace.go | 2 +- .../content/runtime/cli-reference/agh.mdx | 2 + .../cli-reference/onboarding/complete.mdx | 40 ++++++ .../cli-reference/onboarding/index.mdx | 38 ++++++ .../cli-reference/onboarding/meta.json | 4 + .../cli-reference/onboarding/reset.mdx | 40 ++++++ .../cli-reference/onboarding/status.mdx | 50 ++++++++ .../content/runtime/cli-reference/open.mdx | 40 ++++++ .../runtime/cli-reference/scheduler/drain.mdx | 4 - .../runtime/cli-reference/session/index.mdx | 1 + .../runtime/cli-reference/session/meta.json | 1 + .../runtime/cli-reference/session/remove.mdx | 47 ++++++++ .../runtime/cli-reference/task/fail.mdx | 10 +- .../runtime/cli-reference/task/index.mdx | 55 ++++----- .../runtime/cli-reference/task/meta.json | 1 + .../runtime/cli-reference/task/recover.mdx | 49 ++++++++ .../runtime/cli-reference/workspace/index.mdx | 14 +-- .../cli-reference/workspace/remove.mdx | 4 +- .../runtime/core/workspaces/resolver.mdx | 8 +- 25 files changed, 633 insertions(+), 50 deletions(-) create mode 100644 packages/site/content/runtime/cli-reference/onboarding/complete.mdx create mode 100644 packages/site/content/runtime/cli-reference/onboarding/index.mdx create mode 100644 packages/site/content/runtime/cli-reference/onboarding/meta.json create mode 100644 packages/site/content/runtime/cli-reference/onboarding/reset.mdx create mode 100644 packages/site/content/runtime/cli-reference/onboarding/status.mdx create mode 100644 packages/site/content/runtime/cli-reference/open.mdx create mode 100644 packages/site/content/runtime/cli-reference/session/remove.mdx create mode 100644 packages/site/content/runtime/cli-reference/task/recover.mdx diff --git a/internal/api/core/memory_workspace_test.go b/internal/api/core/memory_workspace_test.go index 44d91e43c..2305e711d 100644 --- a/internal/api/core/memory_workspace_test.go +++ b/internal/api/core/memory_workspace_test.go @@ -1142,6 +1142,7 @@ func TestWorkspaceHandlersDelegateToService(t *testing.T) { ListAllFn: func(context.Context) ([]*session.Info, error) { info := testutil.NewSessionInfo("sess-a") info.WorkspaceID = workspace.ID + info.State = session.StateStopped return []*session.Info{info}, nil }, } diff --git a/internal/api/core/workspaces.go b/internal/api/core/workspaces.go index e3381f105..85383db84 100644 --- a/internal/api/core/workspaces.go +++ b/internal/api/core/workspaces.go @@ -225,6 +225,11 @@ func (h *BaseHandlers) DeleteWorkspace(c *gin.Context) { return } + if err := h.deleteStoppedWorkspaceSessions(c.Request.Context(), workspace.ID); err != nil { + h.respondError(c, StatusForWorkspaceError(err), err) + return + } + if err := h.Workspaces.Unregister(c.Request.Context(), workspace.ID); err != nil { h.respondError(c, StatusForWorkspaceError(err), err) return @@ -233,6 +238,54 @@ func (h *BaseHandlers) DeleteWorkspace(c *gin.Context) { c.Status(http.StatusNoContent) } +func (h *BaseHandlers) deleteStoppedWorkspaceSessions(ctx context.Context, workspaceID string) error { + if h.Sessions == nil { + return errors.New("api: session manager is required") + } + + infos, err := h.Sessions.ListAll(ctx) + if err != nil { + return fmt.Errorf("api: list sessions before deleting workspace %q: %w", workspaceID, err) + } + + active := make([]string, 0) + stopped := make([]string, 0) + for _, info := range infos { + if info == nil || strings.TrimSpace(info.WorkspaceID) != workspaceID { + continue + } + sessionID := strings.TrimSpace(info.ID) + if sessionID == "" { + continue + } + if info.State == session.StateActive { + active = append(active, sessionID) + continue + } + stopped = append(stopped, sessionID) + } + if len(active) > 0 { + sort.Strings(active) + return fmt.Errorf( + "api: delete workspace %q: %w: %s", + workspaceID, + workspacepkg.ErrWorkspaceHasActiveSessions, + strings.Join(active, ", "), + ) + } + + sort.Strings(stopped) + for _, sessionID := range stopped { + if err := h.Sessions.Delete(ctx, sessionID); err != nil { + if errors.Is(err, session.ErrSessionNotFound) { + continue + } + return fmt.Errorf("api: delete session %q before workspace %q: %w", sessionID, workspaceID, err) + } + } + return nil +} + // ResolveWorkspace resolves or registers a workspace from a path. func (h *BaseHandlers) ResolveWorkspace(c *gin.Context) { var req contract.ResolveWorkspaceRequest diff --git a/internal/api/httpapi/handlers_error_test.go b/internal/api/httpapi/handlers_error_test.go index 720e62b63..d9aefb73e 100644 --- a/internal/api/httpapi/handlers_error_test.go +++ b/internal/api/httpapi/handlers_error_test.go @@ -198,6 +198,51 @@ func TestDeleteWorkspaceHandlerReturnsConflictWhenWorkspaceHasSessions(t *testin } } +func TestDeleteWorkspaceHandlerReturnsConflictWhenWorkspaceHasActiveSession(t *testing.T) { + t.Parallel() + + t.Run("Should return conflict before unregistering active workspace sessions", func(t *testing.T) { + t.Parallel() + + homePaths := newTestHomePaths(t) + active := newSessionInfo("sess-active") + active.WorkspaceID = "ws_alpha" + active.State = session.StateActive + manager := stubSessionManager{ + ListAllFn: func(context.Context) ([]*session.Info, error) { + return []*session.Info{active}, nil + }, + } + unregisterCalled := false + workspaces := stubWorkspaceService{ + GetFn: func(context.Context, string) (workspacepkg.Workspace, error) { + return workspacepkg.Workspace{ID: "ws_alpha", Name: "alpha"}, nil + }, + UnregisterFn: func(context.Context, string) error { + unregisterCalled = true + return nil + }, + } + engine := newTestRouter( + t, + newTestHandlersWithWorkspace(t, manager, stubObserver{}, workspaces, homePaths), + ) + + resp := performRequest(t, engine, http.MethodDelete, "/api/workspaces/ws_alpha", nil) + if resp.Code != http.StatusConflict { + t.Fatalf( + "delete workspace status = %d, want %d; body=%s", + resp.Code, + http.StatusConflict, + resp.Body.String(), + ) + } + if unregisterCalled { + t.Fatal("Unregister() called despite active workspace session") + } + }) +} + func TestCreateSessionHandlerMapsWorkspaceErrors(t *testing.T) { homePaths := newTestHomePaths(t) manager := stubSessionManager{ diff --git a/internal/api/httpapi/handlers_test.go b/internal/api/httpapi/handlers_test.go index 56189f654..b35484e77 100644 --- a/internal/api/httpapi/handlers_test.go +++ b/internal/api/httpapi/handlers_test.go @@ -1305,6 +1305,63 @@ func TestDeleteWorkspaceHandlerReturnsNoContent(t *testing.T) { } } +func TestDeleteWorkspaceHandlerRemovesStoppedWorkspaceSessions(t *testing.T) { + t.Parallel() + + t.Run("Should delete stopped workspace sessions before unregister", func(t *testing.T) { + t.Parallel() + + homePaths := newTestHomePaths(t) + var deleted []string + manager := stubSessionManager{ + ListAllFn: func(context.Context) ([]*session.Info, error) { + matchingA := newSessionInfo("sess-a") + matchingA.WorkspaceID = "ws_alpha" + matchingA.State = session.StateStopped + matchingB := newSessionInfo("sess-b") + matchingB.WorkspaceID = "ws_alpha" + matchingB.State = session.StateStopped + otherWorkspace := newSessionInfo("sess-other") + otherWorkspace.WorkspaceID = "ws_beta" + otherWorkspace.State = session.StateStopped + return []*session.Info{matchingB, otherWorkspace, matchingA}, nil + }, + DeleteFn: func(_ context.Context, id string) error { + deleted = append(deleted, id) + return nil + }, + } + workspaces := stubWorkspaceService{ + GetFn: func(context.Context, string) (workspacepkg.Workspace, error) { + return workspacepkg.Workspace{ID: "ws_alpha", Name: "alpha"}, nil + }, + UnregisterFn: func(_ context.Context, id string) error { + if id != "ws_alpha" { + t.Fatalf("Unregister() id = %q, want ws_alpha", id) + } + if !slices.Equal(deleted, []string{"sess-a", "sess-b"}) { + t.Fatalf("deleted before unregister = %#v, want sess-a and sess-b", deleted) + } + return nil + }, + } + engine := newTestRouter( + t, + newTestHandlersWithWorkspace(t, manager, stubObserver{}, workspaces, homePaths), + ) + + recorder := performRequest(t, engine, http.MethodDelete, "/api/workspaces/ws_alpha", nil) + if recorder.Code != http.StatusNoContent { + t.Fatalf( + "status = %d, want %d; body=%s", + recorder.Code, + http.StatusNoContent, + recorder.Body.String(), + ) + } + }) +} + func TestResolveWorkspaceHandlerReturnsWorkspace(t *testing.T) { homePaths := newTestHomePaths(t) rootDir := t.TempDir() diff --git a/internal/cli/cli_integration_test.go b/internal/cli/cli_integration_test.go index 66615b277..938677e7f 100644 --- a/internal/cli/cli_integration_test.go +++ b/internal/cli/cli_integration_test.go @@ -36,6 +36,7 @@ import ( sandboxlocal "github.com/compozy/agh/internal/sandbox/local" "github.com/compozy/agh/internal/session" "github.com/compozy/agh/internal/soul" + "github.com/compozy/agh/internal/store" "github.com/compozy/agh/internal/store/globaldb" taskpkg "github.com/compozy/agh/internal/task" workspacepkg "github.com/compozy/agh/internal/workspace" @@ -275,6 +276,119 @@ func TestCLISessionChannelRoundTripIntegration(t *testing.T) { } } +func TestCLISessionRemoveAndWorkspaceRemoveIntegration(t *testing.T) { + t.Parallel() + + t.Run("Should delete session artifacts through daemon routes", func(t *testing.T) { + t.Parallel() + + h := newIntegrationHarness(t) + mustExecuteRoot(t, h.deps, "daemon", "start", "-o", "json") + t.Cleanup(func() { + if _, _, err := executeRootCommand(t, h.deps, "daemon", "stop", "-o", "json"); err != nil { + if !strings.Contains(err.Error(), "daemon is not running") { + t.Errorf("daemon stop cleanup error = %v", err) + } + } + if err := h.runner.waitForExit(); err != nil { + t.Errorf("waitForExit() cleanup error = %v", err) + } + }) + + workspaceOut := mustExecuteRoot(t, h.deps, "workspace", "add", h.workspace, "--name", "alpha", "-o", "json") + var registered WorkspaceRecord + if err := json.Unmarshal([]byte(workspaceOut), ®istered); err != nil { + t.Fatalf("json.Unmarshal(workspace add) error = %v", err) + } + if registered.ID == "" { + t.Fatal("workspace add returned empty id") + } + + active := createIntegrationSession(t, h, "delete-active", "alpha") + activeDir := filepath.Join(h.homePaths.SessionsDir, active.ID) + assertPathExists(t, store.SessionDBFile(activeDir)) + removeOut := mustExecuteRoot(t, h.deps, "session", "remove", active.ID, "-o", "json") + var removed SessionRecord + if err := json.Unmarshal([]byte(removeOut), &removed); err != nil { + t.Fatalf("json.Unmarshal(session remove) error = %v", err) + } + if removed.ID != active.ID { + t.Fatalf("removed.ID = %q, want %q", removed.ID, active.ID) + } + assertPathMissing(t, activeDir) + + cascade := createIntegrationSession(t, h, "workspace-cascade", "alpha") + cascadeStopOut := mustExecuteRoot(t, h.deps, "session", "stop", cascade.ID, "-o", "json") + var stoppedCascade SessionRecord + if err := json.Unmarshal([]byte(cascadeStopOut), &stoppedCascade); err != nil { + t.Fatalf("json.Unmarshal(session stop) error = %v", err) + } + if stoppedCascade.State != session.StateStopped { + t.Fatalf("stoppedCascade.State = %q, want %q", stoppedCascade.State, session.StateStopped) + } + cascadeDir := filepath.Join(h.homePaths.SessionsDir, cascade.ID) + assertPathExists(t, store.SessionDBFile(cascadeDir)) + + workspaceRemoveOut := mustExecuteRoot(t, h.deps, "workspace", "remove", "alpha", "-o", "json") + var removedWorkspace WorkspaceRecord + if err := json.Unmarshal([]byte(workspaceRemoveOut), &removedWorkspace); err != nil { + t.Fatalf("json.Unmarshal(workspace remove) error = %v", err) + } + if removedWorkspace.ID != registered.ID { + t.Fatalf("removedWorkspace.ID = %q, want %q", removedWorkspace.ID, registered.ID) + } + assertPathMissing(t, cascadeDir) + + exitCode, _, stderr := executeRootCommandWithExit(t, h.deps, "session", "status", active.ID, "-o", "json") + if exitCode == 0 { + t.Fatalf("session status after remove exit code = 0, want failure; stderr=%s", stderr) + } + }) +} + +func createIntegrationSession(t *testing.T, h integrationHarness, name string, workspace string) SessionRecord { + t.Helper() + + out := mustExecuteRoot( + t, + h.deps, + "session", + "new", + "--agent", + "coder", + "--name", + name, + "--workspace", + workspace, + "-o", + "json", + ) + var created SessionRecord + if err := json.Unmarshal([]byte(out), &created); err != nil { + t.Fatalf("json.Unmarshal(session new) error = %v", err) + } + if created.ID == "" || created.State != session.StateActive { + t.Fatalf("created session = %#v, want active session with id", created) + } + return created +} + +func assertPathExists(t *testing.T, path string) { + t.Helper() + + if _, err := os.Stat(path); err != nil { + t.Fatalf("Stat(%q) error = %v, want existing path", path, err) + } +} + +func assertPathMissing(t *testing.T, path string) { + t.Helper() + + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("Stat(%q) error = %v, want os.ErrNotExist", path, err) + } +} + func TestCLISessionProviderOverrideIntegration(t *testing.T) { t.Parallel() diff --git a/internal/cli/session.go b/internal/cli/session.go index 7d7d6215e..f540f351f 100644 --- a/internal/cli/session.go +++ b/internal/cli/session.go @@ -231,9 +231,6 @@ func newSessionRemoveCommand(deps commandDeps) *cobra.Command { Use: "remove ", Short: "Remove a session and its persisted history", Example: ` # Remove a stopped session - agh session remove sess_1234 - - # Remove an active session (stops it first) agh session remove sess_1234`, Args: exactOneNonBlankArg(), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/internal/cli/workspace.go b/internal/cli/workspace.go index 2992d9b7d..b6cca73aa 100644 --- a/internal/cli/workspace.go +++ b/internal/cli/workspace.go @@ -321,7 +321,7 @@ func newWorkspaceEditCommand(deps commandDeps) *cobra.Command { func newWorkspaceRemoveCommand(deps commandDeps) *cobra.Command { return &cobra.Command{ Use: "remove ", - Short: "Remove a workspace registration", + Short: "Remove a workspace registration and stopped session history", Example: ` # Remove a workspace registration by name agh workspace remove checkout-api`, Args: exactOneNonBlankArg(), diff --git a/packages/site/content/runtime/cli-reference/agh.mdx b/packages/site/content/runtime/cli-reference/agh.mdx index 6c296d78d..66007d122 100644 --- a/packages/site/content/runtime/cli-reference/agh.mdx +++ b/packages/site/content/runtime/cli-reference/agh.mdx @@ -67,6 +67,8 @@ agh -o json | [agh memory](/runtime/cli-reference/memory) | Show, write, search, and operate Memory v2 durable context | | [agh network](/runtime/cli-reference/network) | Operate the daemon-owned network runtime | | [agh notifications](/runtime/cli-reference/notifications) | Manage notification presets | +| [agh onboarding](/runtime/cli-reference/onboarding) | Inspect and manage first-run onboarding state | +| [agh open](/runtime/cli-reference/open) | Open the AGH web UI in the default browser | | [agh provider](/runtime/cli-reference/provider) | Inspect and manage provider authentication | | [agh resource](/runtime/cli-reference/resource) | Manage desired-state resources | | [agh scheduler](/runtime/cli-reference/scheduler) | Inspect and control task scheduler dispatch | diff --git a/packages/site/content/runtime/cli-reference/onboarding/complete.mdx b/packages/site/content/runtime/cli-reference/onboarding/complete.mdx new file mode 100644 index 000000000..baa167d2b --- /dev/null +++ b/packages/site/content/runtime/cli-reference/onboarding/complete.mdx @@ -0,0 +1,40 @@ +--- +title: "agh onboarding complete" +description: "Mark first-run onboarding as completed" +--- + +## agh onboarding complete + +Mark first-run onboarding as completed + +``` +agh onboarding complete [flags] +``` + +### Options + +``` + -h, --help help for complete +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh onboarding complete -o json +``` diff --git a/packages/site/content/runtime/cli-reference/onboarding/index.mdx b/packages/site/content/runtime/cli-reference/onboarding/index.mdx new file mode 100644 index 000000000..aeee036db --- /dev/null +++ b/packages/site/content/runtime/cli-reference/onboarding/index.mdx @@ -0,0 +1,38 @@ +--- +title: "agh onboarding" +description: "Inspect and manage first-run onboarding state" +--- + +## agh onboarding + +Inspect and manage first-run onboarding state + +### Options + +``` + -h, --help help for onboarding +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +## Subcommands + +| Command | Description | +| --------------------------------------------------------------------- | ------------------------------------------------------------- | +| [agh onboarding complete](/runtime/cli-reference/onboarding/complete) | Mark first-run onboarding as completed | +| [agh onboarding reset](/runtime/cli-reference/onboarding/reset) | Clear the onboarding completion flag so the wizard runs again | +| [agh onboarding status](/runtime/cli-reference/onboarding/status) | Show whether first-run onboarding has been completed | diff --git a/packages/site/content/runtime/cli-reference/onboarding/meta.json b/packages/site/content/runtime/cli-reference/onboarding/meta.json new file mode 100644 index 000000000..3ad306083 --- /dev/null +++ b/packages/site/content/runtime/cli-reference/onboarding/meta.json @@ -0,0 +1,4 @@ +{ + "title": "Onboarding", + "pages": ["index", "complete", "reset", "status"] +} diff --git a/packages/site/content/runtime/cli-reference/onboarding/reset.mdx b/packages/site/content/runtime/cli-reference/onboarding/reset.mdx new file mode 100644 index 000000000..5349468f8 --- /dev/null +++ b/packages/site/content/runtime/cli-reference/onboarding/reset.mdx @@ -0,0 +1,40 @@ +--- +title: "agh onboarding reset" +description: "Clear the onboarding completion flag so the wizard runs again" +--- + +## agh onboarding reset + +Clear the onboarding completion flag so the wizard runs again + +``` +agh onboarding reset [flags] +``` + +### Options + +``` + -h, --help help for reset +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh onboarding reset -o json +``` diff --git a/packages/site/content/runtime/cli-reference/onboarding/status.mdx b/packages/site/content/runtime/cli-reference/onboarding/status.mdx new file mode 100644 index 000000000..ef64d66d0 --- /dev/null +++ b/packages/site/content/runtime/cli-reference/onboarding/status.mdx @@ -0,0 +1,50 @@ +--- +title: "agh onboarding status" +description: "Show whether first-run onboarding has been completed" +--- + +## agh onboarding status + +Show whether first-run onboarding has been completed + +``` +agh onboarding status [flags] +``` + +### Examples + +``` + # Show onboarding status + agh onboarding status + + # Return machine-readable status for agents + agh onboarding status -o json +``` + +### Options + +``` + -h, --help help for status +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh onboarding status -o json +``` diff --git a/packages/site/content/runtime/cli-reference/open.mdx b/packages/site/content/runtime/cli-reference/open.mdx new file mode 100644 index 000000000..a4d1557a4 --- /dev/null +++ b/packages/site/content/runtime/cli-reference/open.mdx @@ -0,0 +1,40 @@ +--- +title: "agh open" +description: "Open the AGH web UI in the default browser" +--- + +## agh open + +Open the AGH web UI in the default browser + +``` +agh open [flags] +``` + +### Options + +``` + -h, --help help for open +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh open -o json +``` diff --git a/packages/site/content/runtime/cli-reference/scheduler/drain.mdx b/packages/site/content/runtime/cli-reference/scheduler/drain.mdx index 6399e665c..baf26d394 100644 --- a/packages/site/content/runtime/cli-reference/scheduler/drain.mdx +++ b/packages/site/content/runtime/cli-reference/scheduler/drain.mdx @@ -7,10 +7,6 @@ description: "Pause dispatch and wait for active task claims to finish" Pause dispatch and wait for active task claims to finish -The v1 command is synchronous: it pauses scheduler dispatch, waits until active claims reach zero -or the timeout expires, and returns one final structured result. It does not open an SSE progress -stream. - ``` agh scheduler drain [flags] ``` diff --git a/packages/site/content/runtime/cli-reference/session/index.mdx b/packages/site/content/runtime/cli-reference/session/index.mdx index 31ef1af6c..a379a601e 100644 --- a/packages/site/content/runtime/cli-reference/session/index.mdx +++ b/packages/site/content/runtime/cli-reference/session/index.mdx @@ -42,6 +42,7 @@ Every AGH command supports `-o, --output`: | [agh session new](/runtime/cli-reference/session/new) | Create a new session | | [agh session prompt](/runtime/cli-reference/session/prompt) | Send a prompt to a session | | [agh session recap](/runtime/cli-reference/session/recap) | Show deterministic session recap | +| [agh session remove](/runtime/cli-reference/session/remove) | Remove a session and its persisted history | | [agh session repair](/runtime/cli-reference/session/repair) | Inspect and repair an interrupted session transcript | | [agh session resume](/runtime/cli-reference/session/resume) | Attach to a resumable session | | [agh session soul](/runtime/cli-reference/session/soul) | Manage session Soul snapshots | diff --git a/packages/site/content/runtime/cli-reference/session/meta.json b/packages/site/content/runtime/cli-reference/session/meta.json index c75132a64..6e10561b5 100644 --- a/packages/site/content/runtime/cli-reference/session/meta.json +++ b/packages/site/content/runtime/cli-reference/session/meta.json @@ -11,6 +11,7 @@ "new", "prompt", "recap", + "remove", "repair", "resume", "status", diff --git a/packages/site/content/runtime/cli-reference/session/remove.mdx b/packages/site/content/runtime/cli-reference/session/remove.mdx new file mode 100644 index 000000000..05f65d2cd --- /dev/null +++ b/packages/site/content/runtime/cli-reference/session/remove.mdx @@ -0,0 +1,47 @@ +--- +title: "agh session remove" +description: "Remove a session and its persisted history" +--- + +## agh session remove + +Remove a session and its persisted history + +``` +agh session remove [flags] +``` + +### Examples + +``` + # Remove a stopped session + agh session remove sess_1234 +``` + +### Options + +``` + -h, --help help for remove +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh session remove -o json +``` diff --git a/packages/site/content/runtime/cli-reference/task/fail.mdx b/packages/site/content/runtime/cli-reference/task/fail.mdx index 4c75d063f..29061775f 100644 --- a/packages/site/content/runtime/cli-reference/task/fail.mdx +++ b/packages/site/content/runtime/cli-reference/task/fail.mdx @@ -1,11 +1,11 @@ --- title: "agh task fail" -description: "Force fail queued or claimed task runs" +description: "Fail task runs through a session-bound lease or operator override" --- ## agh task fail -Force fail queued or claimed task runs +Fail task runs through a session-bound lease or operator override ``` agh task fail [run-id...] [flags] @@ -14,6 +14,9 @@ agh task fail [run-id...] [flags] ### Examples ``` + # Fail the current session's claimed run + agh task fail run-123 --error "provider returned invalid JSON" + # Force fail one run agh task fail run-123 --reason "operator recovery" @@ -26,8 +29,9 @@ agh task fail [run-id...] [flags] ### Options ``` + --error string Session-bound failure message -h, --help help for fail - --metadata string Optional forced-failure metadata JSON + --metadata string Optional failure metadata JSON --reason string Forced-failure reason ``` diff --git a/packages/site/content/runtime/cli-reference/task/index.mdx b/packages/site/content/runtime/cli-reference/task/index.mdx index 5e4d9847e..7c21c5fdc 100644 --- a/packages/site/content/runtime/cli-reference/task/index.mdx +++ b/packages/site/content/runtime/cli-reference/task/index.mdx @@ -54,30 +54,31 @@ agh task -o json ## Subcommands -| Command | Description | -| ----------------------------------------------------------------- | ------------------------------------------------------------- | -| [agh task approve](/runtime/cli-reference/task/approve) | Approve a task and enqueue its first run | -| [agh task cancel](/runtime/cli-reference/task/cancel) | Cancel a task tree | -| [agh task child](/runtime/cli-reference/task/child) | Manage child tasks | -| [agh task complete](/runtime/cli-reference/task/complete) | Complete a claimed task run for the current agent session | -| [agh task create](/runtime/cli-reference/task/create) | Create a task | -| [agh task delete](/runtime/cli-reference/task/delete) | Delete a task | -| [agh task dependency](/runtime/cli-reference/task/dependency) | Manage task dependencies | -| [agh task fail](/runtime/cli-reference/task/fail) | Force fail queued or claimed task runs | -| [agh task get](/runtime/cli-reference/task/get) | Show one task with related detail | -| [agh task heartbeat](/runtime/cli-reference/task/heartbeat) | Extend a claimed task run lease for the current agent session | -| [agh task inspect](/runtime/cli-reference/task/inspect) | Inspect a task or run with diagnostics | -| [agh task list](/runtime/cli-reference/task/list) | List tasks | -| [agh task next](/runtime/cli-reference/task/next) | Claim the next task run for the current agent session | -| [agh task notification](/runtime/cli-reference/task/notification) | Manage task terminal notifications | -| [agh task pause](/runtime/cli-reference/task/pause) | Pause new runs for one task | -| [agh task profile](/runtime/cli-reference/task/profile) | Manage task execution profiles | -| [agh task publish](/runtime/cli-reference/task/publish) | Publish a draft task and enqueue its first run | -| [agh task reject](/runtime/cli-reference/task/reject) | Reject a pending approval task | -| [agh task release](/runtime/cli-reference/task/release) | Force release claimed task runs back to the queue | -| [agh task resume](/runtime/cli-reference/task/resume) | Resume new runs for one paused task | -| [agh task retry](/runtime/cli-reference/task/retry) | Retry one failed task run | -| [agh task review](/runtime/cli-reference/task/review) | Manage task-run reviews | -| [agh task run](/runtime/cli-reference/task/run) | Manage task runs | -| [agh task start](/runtime/cli-reference/task/start) | Enqueue a run for an executable task | -| [agh task update](/runtime/cli-reference/task/update) | Update mutable task fields | +| Command | Description | +| ----------------------------------------------------------------- | ----------------------------------------------------------------- | +| [agh task approve](/runtime/cli-reference/task/approve) | Approve a task and enqueue its first run | +| [agh task cancel](/runtime/cli-reference/task/cancel) | Cancel a task tree | +| [agh task child](/runtime/cli-reference/task/child) | Manage child tasks | +| [agh task complete](/runtime/cli-reference/task/complete) | Complete a claimed task run for the current agent session | +| [agh task create](/runtime/cli-reference/task/create) | Create a task | +| [agh task delete](/runtime/cli-reference/task/delete) | Delete a task | +| [agh task dependency](/runtime/cli-reference/task/dependency) | Manage task dependencies | +| [agh task fail](/runtime/cli-reference/task/fail) | Fail task runs through a session-bound lease or operator override | +| [agh task get](/runtime/cli-reference/task/get) | Show one task with related detail | +| [agh task heartbeat](/runtime/cli-reference/task/heartbeat) | Extend a claimed task run lease for the current agent session | +| [agh task inspect](/runtime/cli-reference/task/inspect) | Inspect a task or run with diagnostics | +| [agh task list](/runtime/cli-reference/task/list) | List tasks | +| [agh task next](/runtime/cli-reference/task/next) | Claim the next task run for the current agent session | +| [agh task notification](/runtime/cli-reference/task/notification) | Manage task terminal notifications | +| [agh task pause](/runtime/cli-reference/task/pause) | Pause new runs for one task | +| [agh task profile](/runtime/cli-reference/task/profile) | Manage task execution profiles | +| [agh task publish](/runtime/cli-reference/task/publish) | Publish a draft task and enqueue its first run | +| [agh task recover](/runtime/cli-reference/task/recover) | Recover one needs_attention task run | +| [agh task reject](/runtime/cli-reference/task/reject) | Reject a pending approval task | +| [agh task release](/runtime/cli-reference/task/release) | Force release claimed task runs back to the queue | +| [agh task resume](/runtime/cli-reference/task/resume) | Resume new runs for one paused task | +| [agh task retry](/runtime/cli-reference/task/retry) | Retry one failed task run | +| [agh task review](/runtime/cli-reference/task/review) | Manage task-run reviews | +| [agh task run](/runtime/cli-reference/task/run) | Manage task runs | +| [agh task start](/runtime/cli-reference/task/start) | Enqueue a run for an executable task | +| [agh task update](/runtime/cli-reference/task/update) | Update mutable task fields | diff --git a/packages/site/content/runtime/cli-reference/task/meta.json b/packages/site/content/runtime/cli-reference/task/meta.json index a78cb0c9e..f02e2429c 100644 --- a/packages/site/content/runtime/cli-reference/task/meta.json +++ b/packages/site/content/runtime/cli-reference/task/meta.json @@ -15,6 +15,7 @@ "next", "pause", "publish", + "recover", "reject", "release", "resume", diff --git a/packages/site/content/runtime/cli-reference/task/recover.mdx b/packages/site/content/runtime/cli-reference/task/recover.mdx new file mode 100644 index 000000000..d5baf5b7b --- /dev/null +++ b/packages/site/content/runtime/cli-reference/task/recover.mdx @@ -0,0 +1,49 @@ +--- +title: "agh task recover" +description: "Recover one needs_attention task run" +--- + +## agh task recover + +Recover one needs_attention task run + +``` +agh task recover [flags] +``` + +### Examples + +``` + # Re-enqueue one run stuck in needs_attention + agh task recover run-123 --reason "operator recovery" +``` + +### Options + +``` + -h, --help help for recover + --metadata string Optional recovery metadata JSON + --reason string Optional recovery reason recorded in the audit event +``` + +### Options inherited from parent commands + +``` + --json Emit JSON output + -o, --output string Output format: human, json, jsonl, or toon (default "human") +``` + +## Output Formats + +Every AGH command supports `-o, --output`: + +- `human` for interactive terminal use +- `json` for scripts and other machine-readable consumers +- `jsonl` for wait or streaming commands that emit one JSON record per line +- `toon` for compact agent-readable summaries + +Example: + +```bash +agh task recover -o json +``` diff --git a/packages/site/content/runtime/cli-reference/workspace/index.mdx b/packages/site/content/runtime/cli-reference/workspace/index.mdx index ba5b2cde0..a306ed851 100644 --- a/packages/site/content/runtime/cli-reference/workspace/index.mdx +++ b/packages/site/content/runtime/cli-reference/workspace/index.mdx @@ -31,10 +31,10 @@ Every AGH command supports `-o, --output`: ## Subcommands -| Command | Description | -| --------------------------------------------------------------- | ---------------------------------------- | -| [agh workspace add](/runtime/cli-reference/workspace/add) | Register a workspace | -| [agh workspace edit](/runtime/cli-reference/workspace/edit) | Edit a registered workspace | -| [agh workspace info](/runtime/cli-reference/workspace/info) | Show one workspace with resolved details | -| [agh workspace list](/runtime/cli-reference/workspace/list) | List registered workspaces | -| [agh workspace remove](/runtime/cli-reference/workspace/remove) | Remove a workspace registration | +| Command | Description | +| --------------------------------------------------------------- | ----------------------------------------------------------- | +| [agh workspace add](/runtime/cli-reference/workspace/add) | Register a workspace | +| [agh workspace edit](/runtime/cli-reference/workspace/edit) | Edit a registered workspace | +| [agh workspace info](/runtime/cli-reference/workspace/info) | Show one workspace with resolved details | +| [agh workspace list](/runtime/cli-reference/workspace/list) | List registered workspaces | +| [agh workspace remove](/runtime/cli-reference/workspace/remove) | Remove a workspace registration and stopped session history | diff --git a/packages/site/content/runtime/cli-reference/workspace/remove.mdx b/packages/site/content/runtime/cli-reference/workspace/remove.mdx index f4bf52a98..4d88d5cba 100644 --- a/packages/site/content/runtime/cli-reference/workspace/remove.mdx +++ b/packages/site/content/runtime/cli-reference/workspace/remove.mdx @@ -1,11 +1,11 @@ --- title: "agh workspace remove" -description: "Remove a workspace registration" +description: "Remove a workspace registration and stopped session history" --- ## agh workspace remove -Remove a workspace registration +Remove a workspace registration and stopped session history ``` agh workspace remove [flags] diff --git a/packages/site/content/runtime/core/workspaces/resolver.mdx b/packages/site/content/runtime/core/workspaces/resolver.mdx index efea5c440..1b484d377 100644 --- a/packages/site/content/runtime/core/workspaces/resolver.mdx +++ b/packages/site/content/runtime/core/workspaces/resolver.mdx @@ -27,9 +27,11 @@ travel with it. `agh workspace add` registers a directory. It does not create or own the directory, and `agh - workspace remove` only removes the registration. Your project files stay on disk. AGH creates - `/.agh/workspace.toml` on first daemon touch when `[memory.workspace] auto_create = - true`; commit it to git so collaborators share the same workspace identity. + workspace remove` leaves your project files on disk. It removes the registration and AGH's + persisted history for stopped sessions in that workspace; active sessions must be stopped or + removed first. AGH creates `/.agh/workspace.toml` on first daemon touch when + `[memory.workspace] auto_create = true`; commit it to git so collaborators share the same + workspace identity. On a fresh install, daemon boot registers the operator home directory (`$HOME`) as the default From cbb0818c574e31b8e3f40b927a136ed25991bf3c Mon Sep 17 00:00:00 2001 From: Pedro Nauck Date: Tue, 2 Jun 2026 12:12:22 -0300 Subject: [PATCH 5/5] fix: review round --- internal/api/core/workspaces.go | 32 ++++++++++++++---- internal/api/httpapi/handlers_error_test.go | 36 +++++++++++++++++++-- internal/api/httpapi/handlers_test.go | 20 ++++++++---- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/internal/api/core/workspaces.go b/internal/api/core/workspaces.go index 85383db84..520eac74d 100644 --- a/internal/api/core/workspaces.go +++ b/internal/api/core/workspaces.go @@ -225,7 +225,8 @@ func (h *BaseHandlers) DeleteWorkspace(c *gin.Context) { return } - if err := h.deleteStoppedWorkspaceSessions(c.Request.Context(), workspace.ID); err != nil { + stoppedSessionIDs, err := h.stoppedWorkspaceSessionIDs(c.Request.Context(), workspace.ID) + if err != nil { h.respondError(c, StatusForWorkspaceError(err), err) return } @@ -235,17 +236,22 @@ func (h *BaseHandlers) DeleteWorkspace(c *gin.Context) { return } + if err := h.deleteStoppedWorkspaceSessions(c.Request.Context(), workspace.ID, stoppedSessionIDs); err != nil { + h.respondError(c, StatusForWorkspaceError(err), err) + return + } + c.Status(http.StatusNoContent) } -func (h *BaseHandlers) deleteStoppedWorkspaceSessions(ctx context.Context, workspaceID string) error { +func (h *BaseHandlers) stoppedWorkspaceSessionIDs(ctx context.Context, workspaceID string) ([]string, error) { if h.Sessions == nil { - return errors.New("api: session manager is required") + return nil, errors.New("api: session manager is required") } infos, err := h.Sessions.ListAll(ctx) if err != nil { - return fmt.Errorf("api: list sessions before deleting workspace %q: %w", workspaceID, err) + return nil, fmt.Errorf("api: list sessions before deleting workspace %q: %w", workspaceID, err) } active := make([]string, 0) @@ -266,7 +272,7 @@ func (h *BaseHandlers) deleteStoppedWorkspaceSessions(ctx context.Context, works } if len(active) > 0 { sort.Strings(active) - return fmt.Errorf( + return nil, fmt.Errorf( "api: delete workspace %q: %w: %s", workspaceID, workspacepkg.ErrWorkspaceHasActiveSessions, @@ -275,12 +281,24 @@ func (h *BaseHandlers) deleteStoppedWorkspaceSessions(ctx context.Context, works } sort.Strings(stopped) - for _, sessionID := range stopped { + return stopped, nil +} + +func (h *BaseHandlers) deleteStoppedWorkspaceSessions( + ctx context.Context, + workspaceID string, + sessionIDs []string, +) error { + if h.Sessions == nil { + return errors.New("api: session manager is required") + } + + for _, sessionID := range sessionIDs { if err := h.Sessions.Delete(ctx, sessionID); err != nil { if errors.Is(err, session.ErrSessionNotFound) { continue } - return fmt.Errorf("api: delete session %q before workspace %q: %w", sessionID, workspaceID, err) + return fmt.Errorf("api: delete session %q after workspace %q: %w", sessionID, workspaceID, err) } } return nil diff --git a/internal/api/httpapi/handlers_error_test.go b/internal/api/httpapi/handlers_error_test.go index d9aefb73e..be6c72147 100644 --- a/internal/api/httpapi/handlers_error_test.go +++ b/internal/api/httpapi/handlers_error_test.go @@ -179,6 +179,19 @@ func TestWorkspaceHandlersReturnExpectedErrors(t *testing.T) { func TestDeleteWorkspaceHandlerReturnsConflictWhenWorkspaceHasSessions(t *testing.T) { homePaths := newTestHomePaths(t) + stopped := newSessionInfo("sess-stopped") + stopped.WorkspaceID = "ws_alpha" + stopped.State = session.StateStopped + var deleted []string + manager := stubSessionManager{ + ListAllFn: func(context.Context) ([]*session.Info, error) { + return []*session.Info{stopped}, nil + }, + DeleteFn: func(_ context.Context, id string) error { + deleted = append(deleted, id) + return nil + }, + } workspaces := stubWorkspaceService{ GetFn: func(context.Context, string) (workspacepkg.Workspace, error) { return workspacepkg.Workspace{ID: "ws_alpha", Name: "alpha"}, nil @@ -189,12 +202,25 @@ func TestDeleteWorkspaceHandlerReturnsConflictWhenWorkspaceHasSessions(t *testin } engine := newTestRouter( t, - newTestHandlersWithWorkspace(t, stubSessionManager{}, stubObserver{}, workspaces, homePaths), + newTestHandlersWithWorkspace(t, manager, stubObserver{}, workspaces, homePaths), ) resp := performRequest(t, engine, http.MethodDelete, "/api/workspaces/ws_alpha", nil) if resp.Code != http.StatusConflict { - t.Fatalf("delete workspace status = %d, want %d", resp.Code, http.StatusConflict) + t.Fatalf( + "delete workspace status = %d, want %d; body=%s", + resp.Code, + http.StatusConflict, + resp.Body.String(), + ) + } + var payload contract.ErrorPayload + decodeJSONResponse(t, resp, &payload) + if payload.Error != workspacepkg.ErrWorkspaceHasSessions.Error() { + t.Fatalf("error = %q, want %q", payload.Error, workspacepkg.ErrWorkspaceHasSessions.Error()) + } + if len(deleted) != 0 { + t.Fatalf("Delete() calls = %#v, want none before failed unregister", deleted) } } @@ -237,6 +263,12 @@ func TestDeleteWorkspaceHandlerReturnsConflictWhenWorkspaceHasActiveSession(t *t resp.Body.String(), ) } + var payload contract.ErrorPayload + decodeJSONResponse(t, resp, &payload) + expectedError := "api: delete workspace \"ws_alpha\": workspace has active sessions: sess-active" + if payload.Error != expectedError { + t.Fatalf("error = %q, want %q", payload.Error, expectedError) + } if unregisterCalled { t.Fatal("Unregister() called despite active workspace session") } diff --git a/internal/api/httpapi/handlers_test.go b/internal/api/httpapi/handlers_test.go index b35484e77..18583b2fd 100644 --- a/internal/api/httpapi/handlers_test.go +++ b/internal/api/httpapi/handlers_test.go @@ -1303,16 +1303,19 @@ func TestDeleteWorkspaceHandlerReturnsNoContent(t *testing.T) { if recorder.Code != http.StatusNoContent { t.Fatalf("status = %d, want %d; body=%s", recorder.Code, http.StatusNoContent, recorder.Body.String()) } + if recorder.Body.Len() != 0 { + t.Fatalf("body = %q, want empty", recorder.Body.String()) + } } func TestDeleteWorkspaceHandlerRemovesStoppedWorkspaceSessions(t *testing.T) { t.Parallel() - t.Run("Should delete stopped workspace sessions before unregister", func(t *testing.T) { + t.Run("Should delete stopped workspace sessions after unregister", func(t *testing.T) { t.Parallel() homePaths := newTestHomePaths(t) - var deleted []string + var calls []string manager := stubSessionManager{ ListAllFn: func(context.Context) ([]*session.Info, error) { matchingA := newSessionInfo("sess-a") @@ -1327,7 +1330,7 @@ func TestDeleteWorkspaceHandlerRemovesStoppedWorkspaceSessions(t *testing.T) { return []*session.Info{matchingB, otherWorkspace, matchingA}, nil }, DeleteFn: func(_ context.Context, id string) error { - deleted = append(deleted, id) + calls = append(calls, "delete:"+id) return nil }, } @@ -1339,9 +1342,7 @@ func TestDeleteWorkspaceHandlerRemovesStoppedWorkspaceSessions(t *testing.T) { if id != "ws_alpha" { t.Fatalf("Unregister() id = %q, want ws_alpha", id) } - if !slices.Equal(deleted, []string{"sess-a", "sess-b"}) { - t.Fatalf("deleted before unregister = %#v, want sess-a and sess-b", deleted) - } + calls = append(calls, "unregister:"+id) return nil }, } @@ -1359,6 +1360,13 @@ func TestDeleteWorkspaceHandlerRemovesStoppedWorkspaceSessions(t *testing.T) { recorder.Body.String(), ) } + if recorder.Body.Len() != 0 { + t.Fatalf("body = %q, want empty", recorder.Body.String()) + } + expectedCalls := []string{"unregister:ws_alpha", "delete:sess-a", "delete:sess-b"} + if !slices.Equal(calls, expectedCalls) { + t.Fatalf("calls = %#v, want %#v", calls, expectedCalls) + } }) }