diff --git a/backend/internal/cli/preview.go b/backend/internal/cli/preview.go index c81cf06ad..527efb308 100644 --- a/backend/internal/cli/preview.go +++ b/backend/internal/cli/preview.go @@ -22,7 +22,12 @@ func newPreviewCommand(ctx *commandContext) *cobra.Command { cmd := &cobra.Command{ Use: "preview [url]", Short: "Open a URL (or the workspace's index.html) in the desktop browser panel for the current session", - Args: cobra.MaximumNArgs(1), + Long: "Open a URL in the desktop browser panel for the current session.\n\n" + + "With no argument it re-opens whatever this session last previewed,\n" + + "falling back to the workspace's index.html. A workspace-relative path\n" + + "(e.g. ./dist/index.html) is served as a local file. Use `ao preview\n" + + "clear` to empty the panel.", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { var target string if len(args) == 1 { @@ -31,17 +36,41 @@ func newPreviewCommand(ctx *commandContext) *cobra.Command { return ctx.openPreview(cmd.Context(), target) }, } + cmd.AddCommand(&cobra.Command{ + Use: "clear", + Short: "Clear the desktop browser panel for the current session", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return ctx.clearPreview(cmd.Context()) + }, + }) return cmd } func (c *commandContext) openPreview(ctx context.Context, target string) error { - sessionID := strings.TrimSpace(os.Getenv("AO_SESSION_ID")) - if sessionID == "" { - return usageError{errors.New("ao preview must run inside an AO session (AO_SESSION_ID is not set)")} + path, err := sessionPreviewPath() + if err != nil { + return err } - - // PathEscape: session ids are already "-"/digit safe, but keep the URL - // well-formed regardless. - path := "sessions/" + url.PathEscape(sessionID) + "/preview" return c.postJSON(ctx, path, previewAPIRequest{Url: target}, nil) } + +// clearPreview empties the desktop browser panel for the current session +// (`ao preview clear`) by deleting the session's stored preview target. +func (c *commandContext) clearPreview(ctx context.Context) error { + path, err := sessionPreviewPath() + if err != nil { + return err + } + return c.deleteJSON(ctx, path, nil) +} + +func sessionPreviewPath() (string, error) { + sessionID := strings.TrimSpace(os.Getenv("AO_SESSION_ID")) + if sessionID == "" { + return "", usageError{errors.New("ao preview must run inside an AO session (AO_SESSION_ID is not set)")} + } + // PathEscape: session ids are already "-"/digit safe, but keep the URL + // well-formed regardless. + return "sessions/" + url.PathEscape(sessionID) + "/preview", nil +} diff --git a/backend/internal/cli/preview_test.go b/backend/internal/cli/preview_test.go index b5dfaabcb..5da492ba8 100644 --- a/backend/internal/cli/preview_test.go +++ b/backend/internal/cli/preview_test.go @@ -14,16 +14,17 @@ import ( type previewCapture struct { body string path string + method string called bool } -// previewServer wires an httptest server expecting -// POST /api/v1/sessions/{id}/preview and captures what the CLI sent. +// previewServer wires an httptest server expecting POST or DELETE on +// /api/v1/sessions/{id}/preview and captures what the CLI sent. func previewServer(t *testing.T, status int, respBody string) (*httptest.Server, *previewCapture) { t.Helper() capture := &previewCapture{} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { + if r.Method != http.MethodPost && r.Method != http.MethodDelete { http.NotFound(w, r) return } @@ -38,6 +39,7 @@ func previewServer(t *testing.T, status int, respBody string) (*httptest.Server, capture.called = true capture.body = string(body) capture.path = r.URL.Path + capture.method = r.Method w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) _, _ = io.WriteString(w, respBody) @@ -89,6 +91,46 @@ func TestPreview_NoArgPostsEmptyURL(t *testing.T) { } } +func TestPreviewClear_DeletesSessionPreview(t *testing.T) { + t.Setenv("AO_SESSION_ID", "aa-47") + cfg := setConfigEnv(t) + srv, capture := previewServer(t, http.StatusOK, `{"ok":true}`) + writeRunFileFor(t, cfg, srv) + + _, errOut, err := executeCLI(t, Deps{ + ProcessAlive: func(int) bool { return true }, + }, "preview", "clear") + if err != nil { + t.Fatalf("unexpected error: %v\nstderr=%s", err, errOut) + } + if capture.method != http.MethodDelete { + t.Errorf("method = %q, want DELETE", capture.method) + } + if capture.path != "/api/v1/sessions/aa-47/preview" { + t.Errorf("path = %q, want /api/v1/sessions/aa-47/preview", capture.path) + } +} + +func TestPreviewClear_MissingSessionIDIsUsageError(t *testing.T) { + t.Setenv("AO_SESSION_ID", "") + cfg := setConfigEnv(t) + srv, capture := previewServer(t, http.StatusOK, `{"ok":true}`) + writeRunFileFor(t, cfg, srv) + + _, _, err := executeCLI(t, Deps{ + ProcessAlive: func(int) bool { return true }, + }, "preview", "clear") + if err == nil { + t.Fatal("expected usage error when AO_SESSION_ID is unset") + } + if got := ExitCode(err); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } + if capture.called { + t.Fatal("daemon should not be contacted when AO_SESSION_ID is unset") + } +} + func TestPreview_MissingSessionIDIsUsageError(t *testing.T) { t.Setenv("AO_SESSION_ID", "") cfg := setConfigEnv(t) diff --git a/backend/internal/domain/session.go b/backend/internal/domain/session.go index 79f94a0d2..f575e8c33 100644 --- a/backend/internal/domain/session.go +++ b/backend/internal/domain/session.go @@ -34,6 +34,10 @@ type SessionMetadata struct { // session. Set via `ao preview` (POST /sessions/{id}/preview); persisted so // it survives a daemon restart. Empty means no preview has been requested. PreviewURL string `json:"previewUrl,omitempty"` + // PreviewRevision is a monotonic counter bumped on every `ao preview` call, + // even when PreviewURL is unchanged. The desktop browser panel keys + // navigation on it so a repeated `ao preview ` still refreshes. + PreviewRevision int64 `json:"previewRevision,omitempty"` } // SessionRecord is the persistence shape. It intentionally stores only durable diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 95ee8c811..a66191e55 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -939,6 +939,44 @@ paths: tags: - sessions /api/v1/sessions/{sessionId}/preview: + delete: + operationId: clearSessionPreview + parameters: + - description: Session identifier, e.g. project-1. + in: path + name: sessionId + required: true + schema: + description: Session identifier, e.g. project-1. + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/SessionResponse' + description: OK + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Found + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Internal Server Error + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Clear the browser preview URL for a session + tags: + - sessions get: operationId: getSessionPreview parameters: @@ -1482,6 +1520,9 @@ components: type: string kind: type: string + previewRevision: + format: int64 + type: integer previewUrl: type: string projectId: diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index c0383858c..a2611d61f 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -506,6 +506,17 @@ func sessionOperations() []operation { {http.StatusNotImplemented, envelope.APIError{}}, }, }, + { + method: http.MethodDelete, path: "/api/v1/sessions/{sessionId}/preview", id: "clearSessionPreview", tag: "sessions", + summary: "Clear the browser preview URL for a session", + pathParams: []any{controllers.SessionIDParam{}}, + resps: []respUnit{ + {http.StatusOK, controllers.SessionResponse{}}, + {http.StatusNotFound, envelope.APIError{}}, + {http.StatusInternalServerError, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, { method: http.MethodGet, path: "/api/v1/sessions/{sessionId}/preview/files/*", id: "getSessionPreviewFile", tag: "sessions", summary: "Serve a static browser preview file from a session workspace", diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 6a0401540..16463bac1 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -123,8 +123,13 @@ type SessionView struct { // PreviewURL is the browser preview target the desktop app opens for this // session, set via POST /sessions/{sessionId}/preview. Empty (omitted) when // no preview has been requested. Pulled from the json:"-" domain Metadata. - PreviewURL string `json:"previewUrl,omitempty"` - PRs []SessionPRFacts `json:"prs"` + PreviewURL string `json:"previewUrl,omitempty"` + // PreviewRevision bumps on every `ao preview` call (even when previewUrl is + // unchanged) so the desktop browser panel can re-navigate / refresh on a + // repeated preview of the same target. Pulled from the json:"-" domain + // Metadata. + PreviewRevision int64 `json:"previewRevision,omitempty"` + PRs []SessionPRFacts `json:"prs"` } // ListSessionsResponse is the body of GET /api/v1/sessions. diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index f676219ff..31d899ad4 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -66,6 +66,7 @@ func (c *SessionsController) Register(r chi.Router) { r.Get("/sessions/{sessionId}", c.get) r.Get("/sessions/{sessionId}/preview", c.preview) r.Post("/sessions/{sessionId}/preview", c.setPreview) + r.Delete("/sessions/{sessionId}/preview", c.clearPreview) r.Get("/sessions/{sessionId}/preview/files/*", c.previewFile) r.Get("/sessions/{sessionId}/pr", c.listPRs) r.Post("/sessions/{sessionId}/pr/claim", c.claimPR) @@ -178,10 +179,19 @@ func (c *SessionsController) previewFile(w http.ResponseWriter, r *http.Request) } // setPreview persists the browser preview URL the desktop app opens for a -// session. An explicit url is used verbatim; an empty url autodetects a static -// entry point in the session workspace (index.html and friends) and serves it -// through the preview/files route. The resolved URL is persisted and fans out a -// session_updated CDC event so the dashboard's browser panel reacts live. +// session and fans out a session_updated CDC event so the dashboard's browser +// panel reacts live. The target is resolved as follows: +// +// - An empty url reuses the session's existing preview target (so a bare +// `ao preview` re-opens whatever this agent/context last previewed), +// falling back to autodetecting a static entry point (index.html and +// friends) only when nothing has been previewed yet. +// - An explicit workspace-local path (e.g. `index.html`, `./dist/index.html`) +// is served through the preview/files route so local files load. +// - Anything else (http(s)/file URLs, host:port dev servers) is kept verbatim. +// +// Every call bumps the session's preview revision, so re-running `ao preview` +// with the same target still refreshes the panel. func (c *SessionsController) setPreview(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { apispec.NotImplemented(w, r, "POST", "/api/v1/sessions/{sessionId}/preview") @@ -193,7 +203,7 @@ func (c *SessionsController) setPreview(w http.ResponseWriter, r *http.Request) return } // Get first so a missing session is rejected with the normal 404 before any - // write, and so autodetect has the workspace path to probe. + // write, and so autodetect/local resolution has the workspace path to probe. sess, err := c.Svc.Get(r.Context(), sessionID(r)) if err != nil { envelope.WriteError(w, r, err) @@ -202,12 +212,16 @@ func (c *SessionsController) setPreview(w http.ResponseWriter, r *http.Request) // ponytail: no URL sanitization on preview target; agent-trusted for now previewURL := strings.TrimSpace(in.URL) if previewURL == "" { - entry, ok := discoverPreviewEntry(sess.Metadata.WorkspacePath) - if !ok { + if existing := strings.TrimSpace(sess.Metadata.PreviewURL); existing != "" { + previewURL = existing + } else if entry, ok := discoverPreviewEntry(sess.Metadata.WorkspacePath); ok { + previewURL = previewFileURL(r, sessionID(r), entry) + } else { envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "NO_PREVIEW_ENTRY", "No preview entry point found in session workspace", nil) return } - previewURL = previewFileURL(r, sessionID(r), entry) + } else if resolved, ok := resolveLocalPreview(r, sessionID(r), sess.Metadata.WorkspacePath, previewURL); ok { + previewURL = resolved } updated, err := c.Svc.SetPreview(r.Context(), sessionID(r), previewURL) if err != nil { @@ -217,6 +231,24 @@ func (c *SessionsController) setPreview(w http.ResponseWriter, r *http.Request) envelope.WriteJSON(w, http.StatusOK, SessionResponse{Session: sessionView(updated)}) } +// clearPreview resets a session's browser preview to empty (`ao preview +// clear`). Unlike setPreview with an empty url it never autodetects: it persists +// an empty target so the desktop browser panel returns to its blank state. The +// write still bumps the preview revision, so the panel hears the change over +// CDC even though the url field is now empty. +func (c *SessionsController) clearPreview(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "DELETE", "/api/v1/sessions/{sessionId}/preview") + return + } + updated, err := c.Svc.SetPreview(r.Context(), sessionID(r), "") + if err != nil { + envelope.WriteError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, SessionResponse{Session: sessionView(updated)}) +} + func (c *SessionsController) listPRs(w http.ResponseWriter, r *http.Request) { if c.Svc == nil { apispec.NotImplemented(w, r, "GET", "/api/v1/sessions/{sessionId}/pr") @@ -528,6 +560,47 @@ func discoverPreviewEntry(workspacePath string) (string, bool) { return "", false } +// resolveLocalPreview maps a workspace-local path (e.g. "index.html" or +// "./dist/index.html") to its preview/files proxy URL when the path resolves to +// a regular file inside the session workspace. It returns ok=false for anything +// that already looks like a URL (an http(s)/file scheme, or a host:port dev +// server) and for paths that escape the workspace or do not point at a file, so +// the caller keeps those targets verbatim. +func resolveLocalPreview(r *http.Request, id domain.SessionID, workspacePath, raw string) (string, bool) { + raw = strings.TrimSpace(raw) + if raw == "" || hasURLScheme(raw) { + return "", false + } + file, ok := confinedPreviewPath(workspacePath, raw) + if !ok { + return "", false + } + info, err := os.Stat(file) + if err != nil || info.IsDir() { + return "", false + } + entry := strings.TrimPrefix(path.Clean("/"+raw), "/") + return previewFileURL(r, id, entry), true +} + +// hasURLScheme reports whether raw begins with an RFC-3986 "scheme:" prefix +// (http:, https:, file:, or a host:port like localhost:5173). It mirrors the +// renderer's withDefaultScheme heuristic so the daemon and browser panel agree +// on what counts as a URL versus a workspace-relative path. +func hasURLScheme(raw string) bool { + for i := 0; i < len(raw); i++ { + c := raw[i] + if c == ':' { + return i > 0 + } + isSchemeChar := c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '+' || c == '.' || c == '-' + if !isSchemeChar { + return false + } + } + return false +} + func confinedPreviewPath(workspacePath, assetPath string) (string, bool) { root, err := filepath.Abs(workspacePath) if err != nil || root == "" { @@ -567,7 +640,7 @@ func escapePath(raw string) string { } func sessionView(s domain.Session) SessionView { - return SessionView{Session: s, Branch: s.Metadata.Branch, PreviewURL: s.Metadata.PreviewURL, PRs: sessionPRFacts(s.PRs)} + return SessionView{Session: s, Branch: s.Metadata.Branch, PreviewURL: s.Metadata.PreviewURL, PreviewRevision: s.Metadata.PreviewRevision, PRs: sessionPRFacts(s.PRs)} } func sessionViews(sessions []domain.Session) []SessionView { diff --git a/backend/internal/httpd/controllers/sessions_test.go b/backend/internal/httpd/controllers/sessions_test.go index f23219387..1cafa64f1 100644 --- a/backend/internal/httpd/controllers/sessions_test.go +++ b/backend/internal/httpd/controllers/sessions_test.go @@ -95,6 +95,9 @@ func (f *fakeSessionService) SetPreview(_ context.Context, id domain.SessionID, return domain.Session{}, apierr.NotFound("SESSION_NOT_FOUND", "Unknown session") } s.Metadata.PreviewURL = previewURL + // Mirror the store: every set bumps the revision, even when the URL is + // unchanged, so the controller's refresh contract can be exercised here. + s.Metadata.PreviewRevision++ f.sessions[id] = s return s, nil } @@ -427,6 +430,131 @@ func TestSessionsAPI_SetPreviewEmptyURLAutodetectsIndex(t *testing.T) { } } +func TestSessionsAPI_SetPreviewEmptyURLReusesExistingTarget(t *testing.T) { + svc := newFakeSessionService() + workspace := t.TempDir() + // An index.html exists, but the session already has a preview target — the + // bare `ao preview` must reuse that target rather than autodetecting index. + if err := os.WriteFile(filepath.Join(workspace, "index.html"), []byte(``), 0o644); err != nil { + t.Fatalf("write index: %v", err) + } + s := svc.sessions["ao-1"] + s.Metadata = domain.SessionMetadata{WorkspacePath: workspace, PreviewURL: "http://localhost:4321/docs"} + svc.sessions["ao-1"] = s + srv := newSessionTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{}`) + if status != http.StatusOK { + t.Fatalf("set preview = %d, want 200; body=%s", status, body) + } + var resp struct { + Session struct { + PreviewURL string `json:"previewUrl"` + } `json:"session"` + } + mustJSON(t, body, &resp) + if resp.Session.PreviewURL != "http://localhost:4321/docs" { + t.Fatalf("response previewUrl = %q, want reused existing target", resp.Session.PreviewURL) + } +} + +func TestSessionsAPI_SetPreviewLocalRelativePathResolvesToFilesURL(t *testing.T) { + svc := newFakeSessionService() + workspace := t.TempDir() + if err := os.MkdirAll(filepath.Join(workspace, "dist"), 0o755); err != nil { + t.Fatalf("mkdir dist: %v", err) + } + if err := os.WriteFile(filepath.Join(workspace, "dist", "index.html"), []byte(``), 0o644); err != nil { + t.Fatalf("write dist index: %v", err) + } + s := svc.sessions["ao-1"] + s.Metadata = domain.SessionMetadata{WorkspacePath: workspace} + svc.sessions["ao-1"] = s + srv := newSessionTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{"url":"./dist/index.html"}`) + if status != http.StatusOK { + t.Fatalf("set preview = %d, want 200; body=%s", status, body) + } + var resp struct { + Session struct { + PreviewURL string `json:"previewUrl"` + } `json:"session"` + } + mustJSON(t, body, &resp) + if !strings.HasSuffix(resp.Session.PreviewURL, "/preview/files/dist/index.html") { + t.Fatalf("response previewUrl = %q, want dist/index.html files URL", resp.Session.PreviewURL) + } + if strings.Contains(resp.Session.PreviewURL, workspace) { + t.Fatalf("preview leaked workspace path: %s", resp.Session.PreviewURL) + } + // The resolved files URL actually serves the local file. + parsed, err := url.Parse(resp.Session.PreviewURL) + if err != nil { + t.Fatalf("parse preview URL: %v", err) + } + fileBody, fileStatus, _ := doRequest(t, srv, "GET", parsed.RequestURI(), "") + if fileStatus != http.StatusOK { + t.Fatalf("serve local file = %d, want 200; body=%s", fileStatus, fileBody) + } +} + +func TestSessionsAPI_SetPreviewBumpsRevisionOnSameURL(t *testing.T) { + svc := newFakeSessionService() + srv := newSessionTestServer(t, svc) + + readRevision := func() int64 { + body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{"url":"http://localhost:5173/"}`) + if status != http.StatusOK { + t.Fatalf("set preview = %d, want 200; body=%s", status, body) + } + var resp struct { + Session struct { + PreviewRevision int64 `json:"previewRevision"` + } `json:"session"` + } + mustJSON(t, body, &resp) + return resp.Session.PreviewRevision + } + first := readRevision() + second := readRevision() + if second <= first { + t.Fatalf("revision did not advance on same-URL re-run: first=%d second=%d", first, second) + } +} + +func TestSessionsAPI_ClearPreviewResetsURL(t *testing.T) { + svc := newFakeSessionService() + s := svc.sessions["ao-1"] + s.Metadata = domain.SessionMetadata{PreviewURL: "http://localhost:5173/"} + svc.sessions["ao-1"] = s + srv := newSessionTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "DELETE", "/api/v1/sessions/ao-1/preview", "") + if status != http.StatusOK { + t.Fatalf("clear preview = %d, want 200; body=%s", status, body) + } + var resp struct { + Session struct { + PreviewURL string `json:"previewUrl"` + } `json:"session"` + } + mustJSON(t, body, &resp) + if resp.Session.PreviewURL != "" { + t.Fatalf("response previewUrl = %q, want empty after clear", resp.Session.PreviewURL) + } + if got := svc.sessions["ao-1"].Metadata.PreviewURL; got != "" { + t.Fatalf("persisted previewUrl = %q, want empty after clear", got) + } +} + +func TestSessionsAPI_ClearPreviewNotFound(t *testing.T) { + srv := newSessionTestServer(t, newFakeSessionService()) + + body, status, _ := doRequest(t, srv, "DELETE", "/api/v1/sessions/missing-1/preview", "") + assertErrorCode(t, body, status, http.StatusNotFound, "SESSION_NOT_FOUND") +} + func TestSessionsAPI_SetPreviewEmptyURLNoEntry(t *testing.T) { svc := newFakeSessionService() s := svc.sessions["ao-1"] diff --git a/backend/internal/storage/sqlite/gen/models.go b/backend/internal/storage/sqlite/gen/models.go index b9e7fbd15..f3ca38655 100644 --- a/backend/internal/storage/sqlite/gen/models.go +++ b/backend/internal/storage/sqlite/gen/models.go @@ -169,6 +169,7 @@ type Session struct { DisplayName string FirstSignalAt sql.NullTime PreviewURL string + PreviewRevision int64 } type SessionWorktree struct { diff --git a/backend/internal/storage/sqlite/gen/sessions.sql.go b/backend/internal/storage/sqlite/gen/sessions.sql.go index f788414d7..44a56fdb7 100644 --- a/backend/internal/storage/sqlite/gen/sessions.sql.go +++ b/backend/internal/storage/sqlite/gen/sessions.sql.go @@ -16,7 +16,7 @@ import ( const getSession = `-- name: GetSession :one SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions WHERE id = ? ` @@ -43,6 +43,7 @@ func (q *Queries) GetSession(ctx context.Context, id domain.SessionID) (Session, &i.DisplayName, &i.FirstSignalAt, &i.PreviewURL, + &i.PreviewRevision, ) return i, err } @@ -52,8 +53,8 @@ INSERT INTO sessions ( id, project_id, num, issue_id, kind, harness, display_name, activity_state, activity_last_at, first_signal_at, is_terminated, branch, workspace_path, runtime_handle_id, agent_session_id, prompt, - preview_url, created_at, updated_at -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + preview_url, preview_revision, created_at, updated_at +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ` type InsertSessionParams struct { @@ -74,6 +75,7 @@ type InsertSessionParams struct { AgentSessionID string Prompt string PreviewURL string + PreviewRevision int64 CreatedAt time.Time UpdatedAt time.Time } @@ -97,6 +99,7 @@ func (q *Queries) InsertSession(ctx context.Context, arg InsertSessionParams) er arg.AgentSessionID, arg.Prompt, arg.PreviewURL, + arg.PreviewRevision, arg.CreatedAt, arg.UpdatedAt, ) @@ -106,7 +109,7 @@ func (q *Queries) InsertSession(ctx context.Context, arg InsertSessionParams) er const listAllSessions = `-- name: ListAllSessions :many SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions ORDER BY project_id, num ` @@ -139,6 +142,7 @@ func (q *Queries) ListAllSessions(ctx context.Context) ([]Session, error) { &i.DisplayName, &i.FirstSignalAt, &i.PreviewURL, + &i.PreviewRevision, ); err != nil { return nil, err } @@ -156,7 +160,7 @@ func (q *Queries) ListAllSessions(ctx context.Context) ([]Session, error) { const listSessionsByProject = `-- name: ListSessionsByProject :many SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions WHERE project_id = ? ORDER BY num ` @@ -189,6 +193,7 @@ func (q *Queries) ListSessionsByProject(ctx context.Context, projectID domain.Pr &i.DisplayName, &i.FirstSignalAt, &i.PreviewURL, + &i.PreviewRevision, ); err != nil { return nil, err } @@ -257,7 +262,7 @@ func (q *Queries) SessionIsSeed(ctx context.Context, id domain.SessionID) (bool, } const setSessionPreviewURL = `-- name: SetSessionPreviewURL :execrows -UPDATE sessions SET preview_url = ?, updated_at = ? WHERE id = ? +UPDATE sessions SET preview_url = ?, preview_revision = preview_revision + 1, updated_at = ? WHERE id = ? ` type SetSessionPreviewURLParams struct { @@ -266,6 +271,9 @@ type SetSessionPreviewURLParams struct { ID domain.SessionID } +// preview_revision is bumped on every call (even when preview_url is unchanged) +// so a repeated `ao preview ` still trips the sessions_cdc_update +// trigger and the desktop browser panel re-navigates / refreshes. func (q *Queries) SetSessionPreviewURL(ctx context.Context, arg SetSessionPreviewURLParams) (int64, error) { result, err := q.db.ExecContext(ctx, setSessionPreviewURL, arg.PreviewURL, arg.UpdatedAt, arg.ID) if err != nil { @@ -279,7 +287,7 @@ UPDATE sessions SET issue_id = ?, kind = ?, harness = ?, display_name = ?, activity_state = ?, activity_last_at = ?, first_signal_at = ?, is_terminated = ?, branch = ?, workspace_path = ?, runtime_handle_id = ?, agent_session_id = ?, prompt = ?, - preview_url = ?, updated_at = ? + preview_url = ?, preview_revision = ?, updated_at = ? WHERE id = ? ` @@ -298,6 +306,7 @@ type UpdateSessionParams struct { AgentSessionID string Prompt string PreviewURL string + PreviewRevision int64 UpdatedAt time.Time ID domain.SessionID } @@ -318,6 +327,7 @@ func (q *Queries) UpdateSession(ctx context.Context, arg UpdateSessionParams) er arg.AgentSessionID, arg.Prompt, arg.PreviewURL, + arg.PreviewRevision, arg.UpdatedAt, arg.ID, ) diff --git a/backend/internal/storage/sqlite/migrations/0019_add_session_preview_revision.sql b/backend/internal/storage/sqlite/migrations/0019_add_session_preview_revision.sql new file mode 100644 index 000000000..57f36027c --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0019_add_session_preview_revision.sql @@ -0,0 +1,55 @@ +-- +goose Up +-- preview_revision is a monotonic counter bumped on every `ao preview` call +-- (POST/DELETE /sessions/{id}/preview). The preview_url alone cannot tell a +-- repeated `ao preview ` from an unrelated session update replayed +-- over CDC, so the desktop browser panel could never refresh on a re-run. The +-- revision gives the renderer a per-command identity to key navigation on, so +-- re-running `ao preview` always re-navigates even when the URL is unchanged. +-- +goose StatementBegin +ALTER TABLE sessions ADD COLUMN preview_revision INTEGER NOT NULL DEFAULT 0; +-- +goose StatementEnd + +-- Recreate the sessions update CDC trigger so a preview_revision bump also fans +-- out a session_updated event. Without this a same-URL `ao preview` re-run +-- would change only preview_revision/updated_at, which the prior trigger did +-- not watch, so the renderer never heard about the refresh. +-- +goose StatementBegin +DROP TRIGGER IF EXISTS sessions_cdc_update; +-- +goose StatementEnd +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_update +AFTER UPDATE ON sessions +WHEN OLD.activity_state <> NEW.activity_state + OR OLD.is_terminated <> NEW.is_terminated + OR (OLD.first_signal_at IS NULL AND NEW.first_signal_at IS NOT NULL) + OR OLD.preview_url <> NEW.preview_url + OR OLD.preview_revision <> NEW.preview_revision +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_updated', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END), 'previewUrl', NEW.preview_url, 'previewRevision', NEW.preview_revision), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP TRIGGER IF EXISTS sessions_cdc_update; +-- +goose StatementEnd +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_update +AFTER UPDATE ON sessions +WHEN OLD.activity_state <> NEW.activity_state + OR OLD.is_terminated <> NEW.is_terminated + OR (OLD.first_signal_at IS NULL AND NEW.first_signal_at IS NOT NULL) + OR OLD.preview_url <> NEW.preview_url +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_updated', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END), 'previewUrl', NEW.preview_url), + NEW.updated_at); +END; +-- +goose StatementEnd +-- +goose StatementBegin +ALTER TABLE sessions DROP COLUMN preview_revision; +-- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/sessions.sql b/backend/internal/storage/sqlite/queries/sessions.sql index 5e44a567e..5c51fe9e3 100644 --- a/backend/internal/storage/sqlite/queries/sessions.sql +++ b/backend/internal/storage/sqlite/queries/sessions.sql @@ -6,33 +6,33 @@ INSERT INTO sessions ( id, project_id, num, issue_id, kind, harness, display_name, activity_state, activity_last_at, first_signal_at, is_terminated, branch, workspace_path, runtime_handle_id, agent_session_id, prompt, - preview_url, created_at, updated_at -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); + preview_url, preview_revision, created_at, updated_at +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); -- name: UpdateSession :exec UPDATE sessions SET issue_id = ?, kind = ?, harness = ?, display_name = ?, activity_state = ?, activity_last_at = ?, first_signal_at = ?, is_terminated = ?, branch = ?, workspace_path = ?, runtime_handle_id = ?, agent_session_id = ?, prompt = ?, - preview_url = ?, updated_at = ? + preview_url = ?, preview_revision = ?, updated_at = ? WHERE id = ?; -- name: GetSession :one SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions WHERE id = ?; -- name: ListSessionsByProject :many SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions WHERE project_id = ? ORDER BY num; -- name: ListAllSessions :many SELECT id, project_id, num, issue_id, kind, harness, activity_state, activity_last_at, is_terminated, branch, workspace_path, - runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url + runtime_handle_id, agent_session_id, prompt, created_at, updated_at, display_name, first_signal_at, preview_url, preview_revision FROM sessions ORDER BY project_id, num; @@ -40,7 +40,10 @@ FROM sessions ORDER BY project_id, num; UPDATE sessions SET display_name = ?, updated_at = ? WHERE id = ?; -- name: SetSessionPreviewURL :execrows -UPDATE sessions SET preview_url = ?, updated_at = ? WHERE id = ?; +-- preview_revision is bumped on every call (even when preview_url is unchanged) +-- so a repeated `ao preview ` still trips the sessions_cdc_update +-- trigger and the desktop browser panel re-navigates / refreshes. +UPDATE sessions SET preview_url = ?, preview_revision = preview_revision + 1, updated_at = ? WHERE id = ?; -- name: SessionIsSeed :one -- SessionIsSeed reports whether the session id matches a row still in seed diff --git a/backend/internal/storage/sqlite/store/session_store.go b/backend/internal/storage/sqlite/store/session_store.go index 03ae4c0e5..351c9adf5 100644 --- a/backend/internal/storage/sqlite/store/session_store.go +++ b/backend/internal/storage/sqlite/store/session_store.go @@ -207,6 +207,7 @@ func rowToRecord(row gen.Session) domain.SessionRecord { AgentSessionID: row.AgentSessionID, Prompt: row.Prompt, PreviewURL: row.PreviewURL, + PreviewRevision: row.PreviewRevision, }, CreatedAt: row.CreatedAt, UpdatedAt: row.UpdatedAt, @@ -233,6 +234,7 @@ func recordToInsert(rec domain.SessionRecord, num int64) gen.InsertSessionParams AgentSessionID: rec.Metadata.AgentSessionID, Prompt: rec.Metadata.Prompt, PreviewURL: rec.Metadata.PreviewURL, + PreviewRevision: rec.Metadata.PreviewRevision, CreatedAt: rec.CreatedAt, UpdatedAt: rec.UpdatedAt, } @@ -256,6 +258,7 @@ func recordToUpdate(rec domain.SessionRecord) gen.UpdateSessionParams { AgentSessionID: rec.Metadata.AgentSessionID, Prompt: rec.Metadata.Prompt, PreviewURL: rec.Metadata.PreviewURL, + PreviewRevision: rec.Metadata.PreviewRevision, UpdatedAt: rec.UpdatedAt, } } diff --git a/backend/internal/storage/sqlite/store/store_test.go b/backend/internal/storage/sqlite/store/store_test.go index 9571ef5ce..830ca88aa 100644 --- a/backend/internal/storage/sqlite/store/store_test.go +++ b/backend/internal/storage/sqlite/store/store_test.go @@ -660,6 +660,50 @@ func TestCDCTriggersPopulateChangeLog(t *testing.T) { } } +func TestSetSessionPreviewURLBumpsRevisionAndFiresCDCOnSameURL(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + r, _ := s.CreateSession(ctx, sampleRecord("mer")) + + base, _ := s.LatestSeq(ctx) + now := time.Now().UTC() + for i := 0; i < 2; i++ { + ok, err := s.SetSessionPreviewURL(ctx, r.ID, "http://localhost:5173/", now.Add(time.Duration(i)*time.Second)) + if err != nil || !ok { + t.Fatalf("set preview url (call %d): ok=%v err=%v", i, ok, err) + } + } + + got, found, err := s.GetSession(ctx, r.ID) + if err != nil || !found { + t.Fatalf("get session: found=%v err=%v", found, err) + } + if got.Metadata.PreviewURL != "http://localhost:5173/" { + t.Fatalf("preview url = %q, want persisted target", got.Metadata.PreviewURL) + } + if got.Metadata.PreviewRevision != 2 { + t.Fatalf("preview revision = %d, want 2 after two sets", got.Metadata.PreviewRevision) + } + + // Both sets fire session_updated even though the URL never changed — the + // revision bump is what trips the trigger, so a same-URL `ao preview` re-run + // still reaches the browser panel. + evs, err := s.EventsAfter(ctx, base, 100) + if err != nil { + t.Fatal(err) + } + updates := 0 + for _, e := range evs { + if string(e.Type) == "session_updated" { + updates++ + } + } + if updates != 2 { + t.Fatalf("session_updated events = %d, want 2 (one per same-URL set)", updates) + } +} + func TestConcurrentSessionCreateAssignsUniqueNums(t *testing.T) { s := newTestStore(t) ctx := context.Background() diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index cde40026e..a7f68302d 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -327,7 +327,8 @@ export interface paths { put?: never; /** Set (or autodetect) the browser preview URL for a session */ post: operations["setSessionPreview"]; - delete?: never; + /** Clear the browser preview URL for a session */ + delete: operations["clearSessionPreview"]; options?: never; head?: never; patch?: never; @@ -524,6 +525,8 @@ export interface components { isTerminated: boolean; issueId?: string; kind: string; + /** Format: int64 */ + previewRevision?: number; previewUrl?: string; projectId: string; prs: components["schemas"]["SessionPRFacts"][]; @@ -2150,6 +2153,56 @@ export interface operations { }; }; }; + clearSessionPreview: { + parameters: { + query?: never; + header?: never; + path: { + /** @description Session identifier, e.g. project-1. */ + sessionId: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["SessionResponse"]; + }; + }; + /** @description Not Found */ + 404: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; getSessionPreviewFile: { parameters: { query?: never; diff --git a/frontend/src/main/browser-view-host.test.ts b/frontend/src/main/browser-view-host.test.ts index 7efff289d..d1e70db20 100644 --- a/frontend/src/main/browser-view-host.test.ts +++ b/frontend/src/main/browser-view-host.test.ts @@ -1,5 +1,64 @@ -import { describe, expect, it } from "vitest"; -import { clampBoundsToWindow, isAllowedBrowserURL, normalizeBrowserURL } from "./browser-view-host"; +import { describe, expect, it, vi } from "vitest"; +import { + type BrowserNavState, + clampBoundsToWindow, + createBrowserViewHost, + isAllowedBrowserURL, + normalizeBrowserURL, +} from "./browser-view-host"; + +type InvokeHandler = (event: unknown, ...args: unknown[]) => unknown; + +function setupHost() { + let currentURL = ""; + const webContents = { + canGoBack: () => false, + canGoForward: () => false, + getTitle: () => "", + getURL: () => currentURL, + goBack: () => undefined, + goForward: () => undefined, + isLoading: () => false, + loadURL: vi.fn(async (url: string) => { + currentURL = url; + }), + on: () => undefined, + reload: () => undefined, + send: () => undefined, + setWindowOpenHandler: () => undefined, + stop: () => undefined, + close: () => undefined, + }; + const view = { + webContents, + setBounds: () => undefined, + setVisible: () => undefined, + }; + const handlers = new Map(); + const sent: BrowserNavState[] = []; + const host = createBrowserViewHost({ + mainWindow: { + contentView: { addChildView: () => undefined, removeChildView: () => undefined }, + getContentBounds: () => ({ x: 0, y: 0, width: 800, height: 600 }), + webContents: { id: 1, send: (_channel: string, state: BrowserNavState) => sent.push(state) }, + } as never, + ipcMain: { + handle: (channel: string, fn: InvokeHandler) => handlers.set(channel, fn), + on: () => undefined, + removeHandler: () => undefined, + off: () => undefined, + } as never, + shell: { openExternal: async () => undefined }, + WebContentsView: function () { + return view; + } as never, + annotatePreloadPath: "/preload.js", + rendererOrigin: "http://localhost:5173", + }); + const invoke = (channel: string, ...args: unknown[]) => + handlers.get(channel)!({ sender: { id: 1 } }, ...args) as Promise; + return { host, invoke, webContents }; +} describe("normalizeBrowserURL", () => { it("defaults localhost-style inputs to http", () => { @@ -33,6 +92,19 @@ describe("isAllowedBrowserURL", () => { }); }); +describe("browser:clear", () => { + it("loads about:blank and reports it as an empty url (cleared state)", async () => { + const { invoke, webContents } = setupHost(); + await invoke("browser:ensure", "sess-1"); + await invoke("browser:navigate", { viewId: "1:sess-1", url: "http://localhost:3000/" }); + + const state = await invoke("browser:clear", "1:sess-1"); + + expect(webContents.loadURL).toHaveBeenLastCalledWith("about:blank"); + expect(state.url).toBe(""); + }); +}); + describe("clampBoundsToWindow", () => { it("rounds and clamps bounds to the window content area", () => { expect( diff --git a/frontend/src/main/browser-view-host.ts b/frontend/src/main/browser-view-host.ts index d49b5a069..e6c373f7d 100644 --- a/frontend/src/main/browser-view-host.ts +++ b/frontend/src/main/browser-view-host.ts @@ -183,6 +183,16 @@ export function createBrowserViewHost(options: BrowserViewHostOptions): BrowserV return pushNavState(options, entry); }; + // clear resets the view to a blank page (`ao preview clear`). about:blank is + // loaded directly, bypassing the URL allowlist — it carries no content and + // readNavState normalizes it back to an empty url so the panel shows its + // empty state. + const clear = async (viewId: string): Promise => { + const entry = ensure(viewId); + await entry.view.webContents.loadURL("about:blank"); + return pushNavState(options, entry); + }; + const destroy = (viewId: string): void => { const entry = entries.get(viewId); if (!entry) return; @@ -215,6 +225,7 @@ export function createBrowserViewHost(options: BrowserViewHostOptions): BrowserV handle("browser:ensure", (event, sessionId: string) => pushNavState(options, ensure(scopedViewId(event, sessionId)))); on("browser:setBounds", (_event, input: BrowserBoundsInput) => setBounds(input)); handle("browser:navigate", (_event, input: BrowserNavigateInput) => navigate(input)); + handle("browser:clear", (_event, viewId: string) => clear(viewId)); handle("browser:goBack", (_event, viewId: string) => invokeNav(viewId, (contents) => contents.goBack())); handle("browser:goForward", (_event, viewId: string) => invokeNav(viewId, (contents) => contents.goForward())); handle("browser:reload", (_event, viewId: string) => invokeNav(viewId, (contents) => contents.reload())); @@ -304,9 +315,13 @@ function pushNavState(options: BrowserViewHostOptions, entry: BrowserEntry): Bro function readNavState(entry: BrowserEntry): BrowserNavState { const { webContents } = entry.view; + const currentURL = webContents.getURL(); return { viewId: entry.state.viewId, - url: webContents.getURL(), + // about:blank is the cleared/blank state — surface it as an empty url so + // the panel renders its "enter a URL" empty state and the address bar is + // blank rather than showing "about:blank". + url: currentURL === "about:blank" ? "" : currentURL, title: webContents.getTitle(), canGoBack: webContents.canGoBack(), canGoForward: webContents.canGoForward(), diff --git a/frontend/src/preload.ts b/frontend/src/preload.ts index 986eb651e..c286a196a 100644 --- a/frontend/src/preload.ts +++ b/frontend/src/preload.ts @@ -43,6 +43,7 @@ const api = { setBounds: (input: BrowserBoundsInput) => ipcRenderer.send("browser:setBounds", input), navigate: (input: BrowserNavigateInput) => ipcRenderer.invoke("browser:navigate", input) as Promise, + clear: (viewId: string) => ipcRenderer.invoke("browser:clear", viewId) as Promise, goBack: (viewId: string) => ipcRenderer.invoke("browser:goBack", viewId) as Promise, goForward: (viewId: string) => ipcRenderer.invoke("browser:goForward", viewId) as Promise, reload: (viewId: string) => ipcRenderer.invoke("browser:reload", viewId) as Promise, diff --git a/frontend/src/renderer/components/BrowserPanel.tsx b/frontend/src/renderer/components/BrowserPanel.tsx index 3cb6580e4..b6d29ce46 100644 --- a/frontend/src/renderer/components/BrowserPanel.tsx +++ b/frontend/src/renderer/components/BrowserPanel.tsx @@ -18,6 +18,7 @@ export function BrowserPanel({ session, active, poppedOut, onTogglePopOut }: Bro active, poppedOut, previewUrl: session.previewUrl, + previewRevision: session.previewRevision, }); return ( (null); + const previewRevision = session?.previewRevision; + const revealedPreviewRef = useRef(null); const browserView = useBrowserView({ sessionId, active: Boolean(session && hasInspector && (browserPoppedOut || isInspectorOpen)), poppedOut: browserPoppedOut, previewUrl, + previewRevision, }); useEffect(() => { @@ -71,14 +73,16 @@ export function SessionView({ sessionId }: SessionViewProps) { // `ao preview` sets session.previewUrl (streamed over CDC); surface the result // in the inspector rail's Browser tab (opening the rail if collapsed), not the - // center pane. Tracked per URL so re-revealing fires on a fresh target but a - // manual tab switch sticks while the URL is unchanged. + // center pane. Tracked per preview revision so re-revealing fires on every + // `ao preview` (even a re-run of the same target) while a manual tab switch + // sticks for a given revision. `ao preview clear` (empty url) does not reveal. useEffect(() => { - if (!previewUrl || revealedPreviewRef.current === previewUrl) return; - revealedPreviewRef.current = previewUrl; + const revision = previewRevision ?? 0; + if (!previewUrl || revealedPreviewRef.current === revision) return; + revealedPreviewRef.current = revision; setInspectorView("browser"); if (!useUiStore.getState().isInspectorOpen) toggleInspector(); - }, [previewUrl, toggleInspector]); + }, [previewRevision, previewUrl, toggleInspector]); // Computed when the inspector panel mounts and frozen while it stays // mounted: rrp re-registers the panel (a layout effect keyed on defaultSize, diff --git a/frontend/src/renderer/hooks/useBrowserView.test.tsx b/frontend/src/renderer/hooks/useBrowserView.test.tsx index db481d53a..ef0e8c9e0 100644 --- a/frontend/src/renderer/hooks/useBrowserView.test.tsx +++ b/frontend/src/renderer/hooks/useBrowserView.test.tsx @@ -47,6 +47,7 @@ function setupBridge() { ), setBounds: vi.fn(), navigate: vi.fn(async ({ viewId }: { viewId: string }) => bridge.stateFor(viewId)), + clear: vi.fn(async (viewId: string) => bridge.stateFor(viewId)), goBack: vi.fn(async (viewId: string) => bridge.stateFor(viewId)), goForward: vi.fn(async (viewId: string) => bridge.stateFor(viewId)), reload: vi.fn(async (viewId: string) => bridge.stateFor(viewId)), @@ -147,11 +148,12 @@ describe("useBrowserView", () => { expect(result.current.navState.title).toBe("Local app"); }); - it("navigates to a daemon-set preview URL and re-navigates only when it changes", async () => { + it("navigates on each preview revision, including a same-URL re-run, and ignores replays", async () => { const bridge = setupBridge(); const { rerender } = renderHook( - ({ previewUrl }) => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false, previewUrl }), - { initialProps: { previewUrl: "http://localhost:5173/" } }, + ({ previewUrl, previewRevision }) => + useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false, previewUrl, previewRevision }), + { initialProps: { previewUrl: "http://localhost:5173/", previewRevision: 1 } }, ); await waitFor(() => @@ -159,22 +161,44 @@ describe("useBrowserView", () => { ); expect(bridge.navigate).toHaveBeenCalledTimes(1); - // Same URL replayed (CDC re-emits the session payload) must not re-navigate. - rerender({ previewUrl: "http://localhost:5173/" }); + // CDC replays the session payload on an unrelated update (revision + // unchanged) — the panel must not reload. + rerender({ previewUrl: "http://localhost:5173/", previewRevision: 1 }); expect(bridge.navigate).toHaveBeenCalledTimes(1); - // A changed target re-navigates. - rerender({ previewUrl: "file:///tmp/preview/index.html" }); + // Re-running `ao preview` with the SAME url bumps the revision and must + // re-navigate (refresh) — the regression this issue fixes. + rerender({ previewUrl: "http://localhost:5173/", previewRevision: 2 }); + await waitFor(() => expect(bridge.navigate).toHaveBeenCalledTimes(2)); + + // A changed target with a fresh revision navigates to the new URL. + rerender({ previewUrl: "file:///tmp/preview/index.html", previewRevision: 3 }); await waitFor(() => expect(bridge.navigate).toHaveBeenCalledWith({ viewId: "42:sess-1", url: "file:///tmp/preview/index.html" }), ); - expect(bridge.navigate).toHaveBeenCalledTimes(2); + expect(bridge.navigate).toHaveBeenCalledTimes(3); }); - it("does not navigate without a preview URL", async () => { + it("clears the view when the preview is reset (ao preview clear) and does not navigate", async () => { + const bridge = setupBridge(); + const { rerender } = renderHook( + ({ previewUrl, previewRevision }) => + useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false, previewUrl, previewRevision }), + { initialProps: { previewUrl: "http://localhost:5173/" as string | undefined, previewRevision: 1 } }, + ); + await waitFor(() => expect(bridge.navigate).toHaveBeenCalledTimes(1)); + + // `ao preview clear` empties previewUrl and bumps the revision. + rerender({ previewUrl: undefined, previewRevision: 2 }); + await waitFor(() => expect(bridge.clear).toHaveBeenCalledWith("42:sess-1")); + expect(bridge.navigate).toHaveBeenCalledTimes(1); + }); + + it("does not navigate or clear without a preview URL at revision zero", async () => { const bridge = setupBridge(); const { result } = renderHook(() => useBrowserView({ sessionId: "sess-1", active: true, poppedOut: false })); await waitFor(() => expect(result.current.viewId).toBe("42:sess-1")); expect(bridge.navigate).not.toHaveBeenCalled(); + expect(bridge.clear).not.toHaveBeenCalled(); }); }); diff --git a/frontend/src/renderer/hooks/useBrowserView.ts b/frontend/src/renderer/hooks/useBrowserView.ts index 3d88e08d7..0788c6c39 100644 --- a/frontend/src/renderer/hooks/useBrowserView.ts +++ b/frontend/src/renderer/hooks/useBrowserView.ts @@ -9,9 +9,16 @@ type UseBrowserViewOptions = { poppedOut: boolean; /** * Preview target driven by the daemon (via `ao preview`, streamed over CDC). - * When set, the view navigates here automatically; changing it re-navigates. + * When set, the view navigates here automatically; an empty value clears it. */ previewUrl?: string; + /** + * Monotonic counter the daemon bumps on every `ao preview` call, even when + * previewUrl is unchanged. The view re-navigates whenever it advances, so a + * repeated `ao preview ` still refreshes (and CDC replays of an + * unrelated session update, which leave it unchanged, are ignored). + */ + previewRevision?: number; }; export type BrowserViewModel = { @@ -37,7 +44,13 @@ const EMPTY_NAV_STATE: BrowserNavState = { const HIDDEN_RECT: BrowserRect = { x: 0, y: 0, width: 0, height: 0 }; -export function useBrowserView({ sessionId, active, poppedOut, previewUrl }: UseBrowserViewOptions): BrowserViewModel { +export function useBrowserView({ + sessionId, + active, + poppedOut, + previewUrl, + previewRevision, +}: UseBrowserViewOptions): BrowserViewModel { const [viewId, setViewId] = useState(""); const [navState, setNavState] = useState(EMPTY_NAV_STATE); const slotNodeRef = useRef(null); @@ -45,7 +58,7 @@ export function useBrowserView({ sessionId, active, poppedOut, previewUrl }: Use const activeRef = useRef(active); const frameRef = useRef(null); const observerRef = useRef(null); - const previewNavRef = useRef(null); + const previewRevisionRef = useRef(null); useEffect(() => { activeRef.current = active; @@ -167,19 +180,26 @@ export function useBrowserView({ sessionId, active, poppedOut, previewUrl }: Use [withView], ); - // Drive navigation from the daemon-set preview URL. Re-navigate only when the - // target actually changes; skip when it already matches what the view shows - // (the CDC stream replays the same session payload on unrelated updates). + const clear = useCallback(() => withView((id) => window.ao!.browser.clear(id)), [withView]); + + // Drive the view from the daemon-set preview target, keyed on the preview + // revision (bumped on every `ao preview` call). Acting on the revision rather + // than the URL means a repeated `ao preview ` still refreshes, an + // `ao preview clear` (empty URL) blanks the view, and CDC replays of + // unrelated session updates (revision unchanged) are ignored — so the panel + // never reloads on an unrelated activity flip. useEffect(() => { + if (!viewId) return; + const revision = previewRevision ?? 0; + if (previewRevisionRef.current === revision) return; + previewRevisionRef.current = revision; const target = previewUrl?.trim(); - if (!target || !viewId) return; - if (previewNavRef.current === target || navState.url === target) { - previewNavRef.current = target; - return; + if (target) { + void navigate(target); + } else if (revision > 0) { + void clear(); } - previewNavRef.current = target; - void navigate(target); - }, [navState.url, navigate, previewUrl, viewId]); + }, [clear, navigate, previewRevision, previewUrl, viewId]); const destroy = useCallback(() => { const id = viewIdRef.current; diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.ts b/frontend/src/renderer/hooks/useWorkspaceQuery.ts index 271e40c7d..0967d8d31 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.ts +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.ts @@ -58,6 +58,7 @@ async function fetchWorkspaces(): Promise { createdAt: session.createdAt, updatedAt: session.updatedAt, previewUrl: session.previewUrl, + previewRevision: session.previewRevision, prs: (session.prs ?? []).map(toPullRequestFacts), })), })); diff --git a/frontend/src/renderer/lib/bridge.ts b/frontend/src/renderer/lib/bridge.ts index ace80279b..840e09899 100644 --- a/frontend/src/renderer/lib/bridge.ts +++ b/frontend/src/renderer/lib/bridge.ts @@ -45,6 +45,14 @@ export const aoBridge: AoBridge = canGoForward: false, isLoading: false, }), + clear: async (viewId: string) => ({ + viewId, + url: "", + title: "", + canGoBack: false, + canGoForward: false, + isLoading: false, + }), goBack: async (viewId: string) => ({ viewId, url: "", diff --git a/frontend/src/renderer/test/setup.ts b/frontend/src/renderer/test/setup.ts index f21772b11..b228fa15e 100644 --- a/frontend/src/renderer/test/setup.ts +++ b/frontend/src/renderer/test/setup.ts @@ -86,6 +86,14 @@ window.ao = { canGoForward: false, isLoading: false, }), + clear: async (viewId: string) => ({ + viewId, + url: "", + title: "", + canGoBack: false, + canGoForward: false, + isLoading: false, + }), goBack: async (viewId: string) => ({ viewId, url: "", diff --git a/frontend/src/renderer/types/workspace.ts b/frontend/src/renderer/types/workspace.ts index 9812031ac..659620021 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -109,6 +109,12 @@ export type WorkspaceSession = { * CDC. When non-empty, the browser panel opens and navigates here. */ previewUrl?: string; + /** + * Monotonic counter the daemon bumps on every `ao preview` call (even when + * previewUrl is unchanged), so the browser panel can re-navigate / refresh on + * a repeated preview of the same target. + */ + previewRevision?: number; /** The session's git diff against its base, when known. */ changedFiles?: ChangedFile[]; /** Pre-filled commit subject for the Git rail, when known. */