Skip to content

Commit 3e3d22c

Browse files
Vaibhaav-TiwariVaibhaavgithub-actions[bot]
authored
fix: make preview browser deterministic (#384)
* feat(frontend): add live browser panel * feat: preserve and auto-open browser previews * fix: retry browser preview after session updates * fix: wait for browser view before preview navigation * fix: reopen preview after session switches * fix: preserve browser views across session switches * test: allow project settings form more time * fix: make preview browser deterministic * chore: format with prettier [skip ci] * fix: preserve cleared preview state --------- Co-authored-by: Vaibhaav <user@example.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 2946212 commit 3e3d22c

11 files changed

Lines changed: 723 additions & 72 deletions

File tree

backend/internal/cli/preview.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ func newPreviewCommand(ctx *commandContext) *cobra.Command {
2323
Use: "preview [url]",
2424
Short: "Open a URL (or the workspace's index.html) in the desktop browser panel for the current session",
2525
Long: "Open a URL in the desktop browser panel for the current session.\n\n" +
26-
"With no argument it re-opens whatever this session last previewed,\n" +
27-
"falling back to the workspace's index.html. A workspace-relative path\n" +
26+
"With no argument it opens the workspace's index.html. A workspace-relative path\n" +
2827
"(e.g. ./dist/index.html) is served as a local file. Use `ao preview\n" +
2928
"clear` to empty the panel.",
3029
Args: cobra.MaximumNArgs(1),

backend/internal/daemon/daemon.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/aoagents/agent-orchestrator/backend/internal/httpd"
1919
"github.com/aoagents/agent-orchestrator/backend/internal/notify"
2020
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
21+
"github.com/aoagents/agent-orchestrator/backend/internal/preview"
2122
"github.com/aoagents/agent-orchestrator/backend/internal/runfile"
2223
notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification"
2324
projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project"
@@ -127,6 +128,7 @@ func Run() error {
127128
}
128129
return fmt.Errorf("wire session service: %w", err)
129130
}
131+
previewDone := preview.NewPoller(store, sessionSvc, "http://"+cfg.Addr(), preview.PollerConfig{Logger: log}).Start(ctx)
130132

131133
srv, err := httpd.NewWithDeps(cfg, log, termMgr, httpd.APIDeps{
132134
Projects: projectsvc.NewWithDeps(projectsvc.Deps{Store: store, Sessions: sessionSvc, Telemetry: telemetrySink}),
@@ -141,6 +143,7 @@ func Run() error {
141143
})
142144
if err != nil {
143145
stop()
146+
<-previewDone
144147
lcStack.Stop()
145148
if cdcErr := cdcPipe.Stop(); cdcErr != nil {
146149
log.Error("cdc pipeline shutdown", "err", cdcErr)
@@ -155,6 +158,7 @@ func Run() error {
155158
// via defer) avoids the LIFO trap where a Stop() that blocks on ctx-cancel
156159
// runs before the cancel — which would hang any non-signal exit path.
157160
stop()
161+
<-previewDone
158162
lcStack.Stop()
159163
if err := cdcPipe.Stop(); err != nil {
160164
log.Error("cdc pipeline shutdown", "err", err)

backend/internal/httpd/controllers/sessions.go

Lines changed: 68 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec"
1818
"github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope"
1919
"github.com/aoagents/agent-orchestrator/backend/internal/ports"
20+
previewutil "github.com/aoagents/agent-orchestrator/backend/internal/preview"
2021
sessionsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/session"
2122
)
2223

@@ -25,6 +26,8 @@ const (
2526
maxMessageLen = 4096
2627
)
2728

29+
var errPreviewFileNotFound = errors.New("preview file not found")
30+
2831
// SessionService is the controller-facing session service contract.
2932
type SessionService interface {
3033
List(ctx context.Context, filter sessionsvc.ListFilter) ([]domain.Session, error)
@@ -182,10 +185,9 @@ func (c *SessionsController) previewFile(w http.ResponseWriter, r *http.Request)
182185
// session and fans out a session_updated CDC event so the dashboard's browser
183186
// panel reacts live. The target is resolved as follows:
184187
//
185-
// - An empty url reuses the session's existing preview target (so a bare
186-
// `ao preview` re-opens whatever this agent/context last previewed),
187-
// falling back to autodetecting a static entry point (index.html and
188-
// friends) only when nothing has been previewed yet.
188+
// - An empty url opens the workspace's static entry point (index.html and
189+
// friends), falling back to the session's existing preview target only
190+
// when no entry point exists.
189191
// - An explicit workspace-local path (e.g. `index.html`, `./dist/index.html`)
190192
// is served through the preview/files route so local files load.
191193
// - Anything else (http(s)/file URLs, host:port dev servers) is kept verbatim.
@@ -212,16 +214,26 @@ func (c *SessionsController) setPreview(w http.ResponseWriter, r *http.Request)
212214
// ponytail: no URL sanitization on preview target; agent-trusted for now
213215
previewURL := strings.TrimSpace(in.URL)
214216
if previewURL == "" {
215-
if existing := strings.TrimSpace(sess.Metadata.PreviewURL); existing != "" {
216-
previewURL = existing
217-
} else if entry, ok := discoverPreviewEntry(sess.Metadata.WorkspacePath); ok {
217+
if entry, ok := discoverPreviewEntry(sess.Metadata.WorkspacePath); ok {
218218
previewURL = previewFileURL(r, sessionID(r), entry)
219+
} else if existing := strings.TrimSpace(sess.Metadata.PreviewURL); existing != "" {
220+
var resolveErr error
221+
previewURL, resolveErr = resolvePreviewTarget(r, sessionID(r), sess.Metadata.WorkspacePath, existing)
222+
if resolveErr != nil {
223+
writePreviewResolveError(w, r, resolveErr)
224+
return
225+
}
219226
} else {
220227
envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "NO_PREVIEW_ENTRY", "No preview entry point found in session workspace", nil)
221228
return
222229
}
223-
} else if resolved, ok := resolveLocalPreview(r, sessionID(r), sess.Metadata.WorkspacePath, previewURL); ok {
224-
previewURL = resolved
230+
} else {
231+
var resolveErr error
232+
previewURL, resolveErr = resolvePreviewTarget(r, sessionID(r), sess.Metadata.WorkspacePath, previewURL)
233+
if resolveErr != nil {
234+
writePreviewResolveError(w, r, resolveErr)
235+
return
236+
}
225237
}
226238
updated, err := c.Svc.SetPreview(r.Context(), sessionID(r), previewURL)
227239
if err != nil {
@@ -544,20 +556,8 @@ func writeSessionPRError(w http.ResponseWriter, r *http.Request, err error) {
544556
}
545557

546558
func discoverPreviewEntry(workspacePath string) (string, bool) {
547-
if strings.TrimSpace(workspacePath) == "" {
548-
return "", false
549-
}
550-
for _, candidate := range []string{"index.html", "public/index.html", "dist/index.html", "build/index.html"} {
551-
file, ok := confinedPreviewPath(workspacePath, candidate)
552-
if !ok {
553-
continue
554-
}
555-
info, err := os.Stat(file)
556-
if err == nil && !info.IsDir() {
557-
return candidate, true
558-
}
559-
}
560-
return "", false
559+
entry, ok := previewutil.DiscoverEntry(workspacePath)
560+
return entry.Path, ok
561561
}
562562

563563
// resolveLocalPreview maps a workspace-local path (e.g. "index.html" or
@@ -583,6 +583,49 @@ func resolveLocalPreview(r *http.Request, id domain.SessionID, workspacePath, ra
583583
return previewFileURL(r, id, entry), true
584584
}
585585

586+
func resolvePreviewTarget(r *http.Request, id domain.SessionID, workspacePath, raw string) (string, error) {
587+
raw = strings.TrimSpace(raw)
588+
if isAbsolutePreviewPath(raw) {
589+
return absolutePreviewFileURL(raw)
590+
}
591+
if resolved, ok := resolveLocalPreview(r, id, workspacePath, raw); ok {
592+
return resolved, nil
593+
}
594+
return raw, nil
595+
}
596+
597+
func isAbsolutePreviewPath(raw string) bool {
598+
return filepath.IsAbs(raw) || isWindowsAbsolutePath(raw)
599+
}
600+
601+
func isWindowsAbsolutePath(raw string) bool {
602+
return len(raw) >= 3 && ((raw[0] >= 'a' && raw[0] <= 'z') || (raw[0] >= 'A' && raw[0] <= 'Z')) && raw[1] == ':' && (raw[2] == '\\' || raw[2] == '/')
603+
}
604+
605+
func absolutePreviewFileURL(raw string) (string, error) {
606+
file, err := filepath.Abs(raw)
607+
if err != nil {
608+
return "", errPreviewFileNotFound
609+
}
610+
info, err := os.Stat(file)
611+
if err != nil || info.IsDir() {
612+
return "", errPreviewFileNotFound
613+
}
614+
filePath := filepath.ToSlash(file)
615+
if filepath.VolumeName(file) != "" || isWindowsAbsolutePath(filePath) {
616+
filePath = "/" + filePath
617+
}
618+
return (&url.URL{Scheme: "file", Path: filePath}).String(), nil
619+
}
620+
621+
func writePreviewResolveError(w http.ResponseWriter, r *http.Request, err error) {
622+
if errors.Is(err, errPreviewFileNotFound) {
623+
envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "PREVIEW_FILE_NOT_FOUND", "Preview file not found", nil)
624+
return
625+
}
626+
envelope.WriteError(w, r, err)
627+
}
628+
586629
// hasURLScheme reports whether raw begins with an RFC-3986 "scheme:" prefix
587630
// (http:, https:, file:, or a host:port like localhost:5173). It mirrors the
588631
// renderer's withDefaultScheme heuristic so the daemon and browser panel agree
@@ -602,41 +645,11 @@ func hasURLScheme(raw string) bool {
602645
}
603646

604647
func confinedPreviewPath(workspacePath, assetPath string) (string, bool) {
605-
root, err := filepath.Abs(workspacePath)
606-
if err != nil || root == "" {
607-
return "", false
608-
}
609-
clean := strings.TrimPrefix(path.Clean("/"+strings.TrimSpace(assetPath)), "/")
610-
if clean == "" || clean == "." {
611-
clean = "index.html"
612-
}
613-
file := filepath.Join(root, filepath.FromSlash(clean))
614-
absFile, err := filepath.Abs(file)
615-
if err != nil {
616-
return "", false
617-
}
618-
rel, err := filepath.Rel(root, absFile)
619-
if err != nil || rel == "." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || rel == ".." {
620-
return "", false
621-
}
622-
return absFile, true
648+
return previewutil.ConfinedPath(workspacePath, assetPath)
623649
}
624650

625651
func previewFileURL(r *http.Request, id domain.SessionID, entry string) string {
626-
u := url.URL{
627-
Scheme: "http",
628-
Host: r.Host,
629-
Path: "/api/v1/sessions/" + url.PathEscape(string(id)) + "/preview/files/" + escapePath(entry),
630-
}
631-
return u.String()
632-
}
633-
634-
func escapePath(raw string) string {
635-
parts := strings.Split(raw, "/")
636-
for i, part := range parts {
637-
parts[i] = url.PathEscape(part)
638-
}
639-
return strings.Join(parts, "/")
652+
return previewutil.FileURL("http://"+r.Host, id, entry)
640653
}
641654

642655
func sessionView(s domain.Session) SessionView {

backend/internal/httpd/controllers/sessions_test.go

Lines changed: 99 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/url"
1010
"os"
1111
"path/filepath"
12+
"strconv"
1213
"strings"
1314
"testing"
1415
"time"
@@ -430,11 +431,11 @@ func TestSessionsAPI_SetPreviewEmptyURLAutodetectsIndex(t *testing.T) {
430431
}
431432
}
432433

433-
func TestSessionsAPI_SetPreviewEmptyURLReusesExistingTarget(t *testing.T) {
434+
func TestSessionsAPI_SetPreviewEmptyURLPrefersWorkspaceEntryOverExistingTarget(t *testing.T) {
434435
svc := newFakeSessionService()
435436
workspace := t.TempDir()
436-
// An index.html exists, but the session already has a preview target — the
437-
// bare `ao preview` must reuse that target rather than autodetecting index.
437+
// An index.html exists, so bare `ao preview` returns to the workspace entry
438+
// instead of sticking to the last explicit target.
438439
if err := os.WriteFile(filepath.Join(workspace, "index.html"), []byte(`<html></html>`), 0o644); err != nil {
439440
t.Fatalf("write index: %v", err)
440441
}
@@ -443,6 +444,57 @@ func TestSessionsAPI_SetPreviewEmptyURLReusesExistingTarget(t *testing.T) {
443444
svc.sessions["ao-1"] = s
444445
srv := newSessionTestServer(t, svc)
445446

447+
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{}`)
448+
if status != http.StatusOK {
449+
t.Fatalf("set preview = %d, want 200; body=%s", status, body)
450+
}
451+
var resp struct {
452+
Session struct {
453+
PreviewURL string `json:"previewUrl"`
454+
} `json:"session"`
455+
}
456+
mustJSON(t, body, &resp)
457+
if !strings.HasSuffix(resp.Session.PreviewURL, "/preview/files/index.html") {
458+
t.Fatalf("response previewUrl = %q, want workspace index files URL", resp.Session.PreviewURL)
459+
}
460+
}
461+
462+
func TestSessionsAPI_SetPreviewEmptyURLNormalizesExistingRelativeTarget(t *testing.T) {
463+
svc := newFakeSessionService()
464+
workspace := t.TempDir()
465+
if err := os.WriteFile(filepath.Join(workspace, "index.html"), []byte(`<html></html>`), 0o644); err != nil {
466+
t.Fatalf("write index: %v", err)
467+
}
468+
s := svc.sessions["ao-1"]
469+
s.Metadata = domain.SessionMetadata{WorkspacePath: workspace, PreviewURL: "index.html"}
470+
svc.sessions["ao-1"] = s
471+
srv := newSessionTestServer(t, svc)
472+
473+
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{}`)
474+
if status != http.StatusOK {
475+
t.Fatalf("set preview = %d, want 200; body=%s", status, body)
476+
}
477+
var resp struct {
478+
Session struct {
479+
PreviewURL string `json:"previewUrl"`
480+
} `json:"session"`
481+
}
482+
mustJSON(t, body, &resp)
483+
if !strings.HasSuffix(resp.Session.PreviewURL, "/preview/files/index.html") {
484+
t.Fatalf("response previewUrl = %q, want index.html files URL", resp.Session.PreviewURL)
485+
}
486+
if got := svc.sessions["ao-1"].Metadata.PreviewURL; got != resp.Session.PreviewURL {
487+
t.Fatalf("persisted previewUrl = %q, want normalized response URL %q", got, resp.Session.PreviewURL)
488+
}
489+
}
490+
491+
func TestSessionsAPI_SetPreviewEmptyURLReusesExistingTargetWhenNoEntryExists(t *testing.T) {
492+
svc := newFakeSessionService()
493+
s := svc.sessions["ao-1"]
494+
s.Metadata = domain.SessionMetadata{WorkspacePath: t.TempDir(), PreviewURL: "http://localhost:4321/docs"}
495+
svc.sessions["ao-1"] = s
496+
srv := newSessionTestServer(t, svc)
497+
446498
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{}`)
447499
if status != http.StatusOK {
448500
t.Fatalf("set preview = %d, want 200; body=%s", status, body)
@@ -499,6 +551,50 @@ func TestSessionsAPI_SetPreviewLocalRelativePathResolvesToFilesURL(t *testing.T)
499551
}
500552
}
501553

554+
func TestSessionsAPI_SetPreviewAbsoluteFilePathPersistsFileURL(t *testing.T) {
555+
svc := newFakeSessionService()
556+
file := filepath.Join(t.TempDir(), "implementation_plan.html")
557+
if err := os.WriteFile(file, []byte(`<html></html>`), 0o644); err != nil {
558+
t.Fatalf("write file: %v", err)
559+
}
560+
srv := newSessionTestServer(t, svc)
561+
562+
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{"url":`+strconv.Quote(file)+`}`)
563+
if status != http.StatusOK {
564+
t.Fatalf("set preview = %d, want 200; body=%s", status, body)
565+
}
566+
var resp struct {
567+
Session struct {
568+
PreviewURL string `json:"previewUrl"`
569+
} `json:"session"`
570+
}
571+
mustJSON(t, body, &resp)
572+
parsed, err := url.Parse(resp.Session.PreviewURL)
573+
if err != nil {
574+
t.Fatalf("parse preview url: %v", err)
575+
}
576+
if parsed.Scheme != "file" {
577+
t.Fatalf("previewUrl = %q, want file URL", resp.Session.PreviewURL)
578+
}
579+
}
580+
581+
func TestSessionsAPI_SetPreviewMissingAbsoluteFilePathFailsWithoutOverwriting(t *testing.T) {
582+
svc := newFakeSessionService()
583+
missing := filepath.Join(t.TempDir(), "implmentation_plan.html")
584+
s := svc.sessions["ao-1"]
585+
s.Metadata = domain.SessionMetadata{PreviewURL: "http://localhost:4321/docs"}
586+
svc.sessions["ao-1"] = s
587+
srv := newSessionTestServer(t, svc)
588+
589+
body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/preview", `{"url":`+strconv.Quote(missing)+`}`)
590+
if status != http.StatusNotFound {
591+
t.Fatalf("set missing absolute preview = %d, want 404; body=%s", status, body)
592+
}
593+
if got := svc.sessions["ao-1"].Metadata.PreviewURL; got != "http://localhost:4321/docs" {
594+
t.Fatalf("persisted previewUrl = %q, want existing target preserved", got)
595+
}
596+
}
597+
502598
func TestSessionsAPI_SetPreviewBumpsRevisionOnSameURL(t *testing.T) {
503599
svc := newFakeSessionService()
504600
srv := newSessionTestServer(t, svc)

0 commit comments

Comments
 (0)