Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions packages/app/src/web/actions-skiller.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { type BrowserActionContext, withBusy } from "./actions-shared.js"
import { openSkiller } from "./api.js"
import { openUrl } from "./open-url.js"
import { type PreparedOpenUrl, prepareOpenUrl } from "./open-url.js"

type SkillerLaunch = {
export type SkillerLaunch = {
readonly alreadyRunning: boolean
readonly appPath: string
readonly logPath: string
Expand All @@ -14,7 +14,7 @@ type SkillerLaunch = {
readonly trpcBasePath: string
}

const skillerLaunchMessage = (launch: SkillerLaunch, openedPath: string, opened: boolean): string => {
export const skillerLaunchMessage = (launch: SkillerLaunch, openedPath: string, opened: boolean): string => {
const pid = launch.pid === null ? "unknown pid" : `pid ${launch.pid}`
const state = launch.alreadyRunning
? `Skiller is already running (${pid}). Log: ${launch.logPath}`
Expand All @@ -27,20 +27,29 @@ const skillerLaunchMessage = (launch: SkillerLaunch, openedPath: string, opened:
: `${state}.${scope} Popup was blocked. Open ${openedPath} manually.`
}

export const openPreparedSkillerLaunch = (launch: SkillerLaunch, preparedUrl: PreparedOpenUrl): string => {
const openedPath = launch.appPath
const opened = preparedUrl.navigate(openedPath)
return skillerLaunchMessage(launch, openedPath, opened)
}

export const openSkillerApp = (
context: BrowserActionContext,
projectKey: string | null | undefined = context.selectedProjectKey,
sessionId?: string
): void => {
const resolvedProjectKey = projectKey ?? undefined
const preparedUrl = prepareOpenUrl()
context.setMessage("Opening Skiller...")
withBusy({
context,
effect: openSkiller(resolvedProjectKey, sessionId),
label: "Opening Skiller",
onFailure: () => {
preparedUrl.close()
},
onSuccess: (launch) => {
const openedPath = launch.appPath
const opened = openUrl(openedPath)
context.setMessage(skillerLaunchMessage(launch, openedPath, opened))
context.setMessage(openPreparedSkillerLaunch(launch, preparedUrl))
}
})
}
22 changes: 22 additions & 0 deletions packages/app/src/web/app-ready-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,27 @@ const StatusHeader = (
</Box>
)

const TerminalWorkspaceStatus = (
{ busyLabel, message }: Pick<ReadyLayoutProps, "busyLabel" | "message">
): JSX.Element | null =>
busyLabel === null && message === null
? null
: (
<Box
backgroundColor="#101317"
border={true}
borderColor="#3a4652"
borderStyle="rounded"
flexShrink={0}
gap="8px"
marginBottom="6px"
padding="6px"
>
<StatusText busyLabel={busyLabel} />
{message === null ? null : <Text fg="#f6d27b" wrap="truncate">message: {message}</Text>}
</Box>
)

export const ReadyLayout = ({ busyLabel, message, ...props }: ReadyLayoutProps): JSX.Element => (
hasVisibleTerminalWorkspace(props)
? (
Expand All @@ -212,6 +233,7 @@ export const ReadyLayout = ({ busyLabel, message, ...props }: ReadyLayoutProps):
padding={terminalWorkspacePadding(props.viewportLayout)}
width="100%"
>
<TerminalWorkspaceStatus busyLabel={busyLabel} message={message} />
<MainPanels {...props} />
</Box>
)
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/web/app-ready-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ const resolveProjectId = (
return project?.id ?? null
}

const activeScreenFromMenu = (menu: BrowserMenuTag, outputRequested: boolean): BrowserScreen => {
export const activeScreenFromMenu = (menu: BrowserMenuTag, outputRequested: boolean): BrowserScreen => {
if (outputRequested && (menu === "Logs" || menu === "Status")) {
return outputScreen()
}
Expand Down
89 changes: 76 additions & 13 deletions packages/app/src/web/app-terminal-session-handlers.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
import { Effect } from "effect"
import { type Dispatch, type SetStateAction, useCallback, useState } from "react"

import { openPreparedSkillerLaunch } from "./actions-skiller.js"
import {
applyProject,
type ContainerTaskSnapshot,
createProjectTerminalSession,
loadProjectBrowser,
loadProjectTaskLogs,
loadProjectTasks,
openSkiller,
projectBrowserCdpUrl,
projectBrowserNoVncUrl,
type ProjectBrowserSession,
stopProjectTask
} from "./api.js"
import { openUrl } from "./open-url.js"
import { openUrl, prepareOpenUrl } from "./open-url.js"
import { projectSshRoutePath } from "./terminal.js"

export type StateMessageUpdater = (message: string | null) => void

export type ProjectHandlers = {
readonly onApplyProject: (() => void) | undefined
readonly onOpenBrowser: (() => void) | undefined
readonly onOpenSkiller: (() => void) | undefined
readonly onOpenTaskManager: (() => void) | undefined
readonly onOpenTerminal: (() => void) | undefined
}
Expand Down Expand Up @@ -116,29 +119,89 @@ const runOpenTerminal = (projectKey: string, setMessage: StateMessageUpdater): v
)
}

const runOpenSkiller = (
projectKey: string,
terminalSessionId: string,
setMessage: StateMessageUpdater
): void => {
const preparedUrl = prepareOpenUrl()
setMessage("Opening Skiller...")
void Effect.runPromise(
openSkiller(projectKey, terminalSessionId).pipe(
Effect.match({
onFailure: (error) => {
preparedUrl.close()
setMessage(`Failed to open Skiller: ${error}`)
},
onSuccess: (launch) => {
setMessage(openPreparedSkillerLaunch(launch, preparedUrl))
}
})
)
)
}

export type ProjectActionHandlersArgs = {
readonly onOpenTaskManagerRequest: () => void
readonly projectId: string | undefined
readonly projectKey: string | undefined
readonly projectLabel: string
readonly setMessage: StateMessageUpdater
readonly terminalSessionId: string | undefined
}

export const useProjectActionHandlers = (
{ onOpenTaskManagerRequest, projectId, projectKey, projectLabel, setMessage }: ProjectActionHandlersArgs
): ProjectHandlers => ({
onApplyProject: projectId === undefined ? undefined : () => {
runApplyProject(projectId, projectLabel, setMessage)
},
onOpenBrowser: projectId === undefined ? undefined : () => {
runOpenBrowser(projectId, setMessage)
},
onOpenTaskManager: projectId === undefined ? undefined : onOpenTaskManagerRequest,
onOpenTerminal: projectId === undefined || projectKey === undefined
const projectAction = (
projectId: string | undefined,
action: (projectId: string) => void
): (() => void) | undefined =>
projectId === undefined
? undefined
: () => {
action(projectId)
}

const projectKeyAction = (
projectId: string | undefined,
projectKey: string | undefined,
action: (projectKey: string) => void
): (() => void) | undefined =>
projectId === undefined || projectKey === undefined
? undefined
: () => {
action(projectKey)
}

const projectTerminalAction = (
projectId: string | undefined,
projectKey: string | undefined,
terminalSessionId: string | undefined,
action: (projectKey: string, terminalSessionId: string) => void
): (() => void) | undefined =>
projectId === undefined || projectKey === undefined || terminalSessionId === undefined
? undefined
: () => {
runOpenTerminal(projectKey, setMessage)
action(projectKey, terminalSessionId)
}

export const useProjectActionHandlers = (
{ onOpenTaskManagerRequest, projectId, projectKey, projectLabel, setMessage, terminalSessionId }:
ProjectActionHandlersArgs
): ProjectHandlers => ({
onApplyProject: projectAction(projectId, (resolvedProjectId) => {
runApplyProject(resolvedProjectId, projectLabel, setMessage)
}),
onOpenBrowser: projectAction(projectId, (resolvedProjectId) => {
runOpenBrowser(resolvedProjectId, setMessage)
}),
onOpenSkiller: projectTerminalAction(projectId, projectKey, terminalSessionId, (resolvedProjectKey, sessionId) => {
runOpenSkiller(resolvedProjectKey, sessionId, setMessage)
}),
onOpenTaskManager: projectAction(projectId, () => {
onOpenTaskManagerRequest()
}),
onOpenTerminal: projectKeyAction(projectId, projectKey, (resolvedProjectKey) => {
runOpenTerminal(resolvedProjectKey, setMessage)
})
})

const runRefreshTasks = (
Expand Down
1 change: 1 addition & 0 deletions packages/app/src/web/app-terminal-session-ui.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export const TerminalOnlyTerminalPanel = (
onKill={callbacks.onKill}
onMessage={callbacks.onMessage}
onOpenBrowser={handlers.onOpenBrowser}
onOpenSkiller={handlers.onOpenSkiller}
onOpenTaskManager={handlers.onOpenTaskManager}
onOpenTerminal={handlers.onOpenTerminal}
session={session}
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/web/app-terminal-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ const useTerminalOnlyReadyState = (
projectId,
projectKey,
projectLabel,
setMessage
setMessage,
terminalSessionId: session.session.id
})
return { handlers, project, setMessage, setTaskManagerOpen, taskManagerOpen, tasks }
}
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/web/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type BrowserMenuTag =
| "Delete"
| "Quit"

const browserMenuOrder: ReadonlyArray<BrowserMenuTag> = [
export const browserMenuOrder: ReadonlyArray<BrowserMenuTag> = [
"Create",
"Select",
"Auth",
Expand Down
31 changes: 31 additions & 0 deletions packages/app/src/web/open-url.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,38 @@
export type PreparedOpenUrl = {
readonly close: () => void
readonly navigate: (url: string) => boolean
}

export const openUrl = (url: string): boolean => {
if (typeof globalThis.open === "function") {
const openedWindow = globalThis.open(url, "_blank", "noopener")
return openedWindow !== null
}
return false
}

const blockedPreparedOpenUrl = (): PreparedOpenUrl => ({
close: () => {},
navigate: openUrl
})

export const prepareOpenUrl = (): PreparedOpenUrl => {
if (typeof globalThis.open !== "function") {
return blockedPreparedOpenUrl()
}
const openedWindow = globalThis.open("about:blank", "_blank", "noopener")
if (openedWindow === null) {
return blockedPreparedOpenUrl()
}
openedWindow.opener = null
return {
close: () => {
openedWindow.close()
},
navigate: (url) => {
openedWindow.location.href = url
openedWindow.focus()
return true
Comment on lines +32 to +35
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

navigate возвращает успех без валидации факта навигации.

На Line 35 всегда возвращается true, из-за чего можно получить ложное "Opened ..." сообщение, хотя переход не состоялся (например, вкладка уже недоступна). Лучше вернуть реальный результат и дать fallback на ручное открытие.

💡 Предлагаемое исправление
     navigate: (url) => {
-      openedWindow.location.href = url
-      openedWindow.focus()
-      return true
+      if (openedWindow.closed) {
+        return openUrl(url)
+      }
+      openedWindow.location.href = url
+      openedWindow.focus()
+      return !openedWindow.closed
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/app/src/web/open-url.ts` around lines 32 - 35, Функция navigate
всегда возвращает true even если навигация не произошла; исправьте вью: перед
возвратом успеха проверяйте состояние целевого окна (используя
openedWindow.closed) и оборачивайте присвоение openedWindow.location.href/focus
в try/catch, возвращая false при исключении или если openedWindow.closed ===
true; при false вызывайте fallback (например открыть URL через window.open или
показать инструкцию пользователю). Обновите логику в методе navigate и там, где
он вызывается, чтобы реагировать на false и предлагать ручное открытие.

}
}
}
6 changes: 4 additions & 2 deletions packages/app/src/web/screen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const outputScreen = (): BrowserScreen => ({ tag: "Output" })

export const projectPickerScreen = (): BrowserScreen => ({ tag: "ProjectPicker" })

const projectMenuTags: ReadonlySet<BrowserMenuTag> = new Set<BrowserMenuTag>([
export const browserProjectMenuTags: ReadonlyArray<BrowserProjectMenuTag> = [
"Browser",
"Databases",
"Delete",
Expand All @@ -46,7 +46,9 @@ const projectMenuTags: ReadonlySet<BrowserMenuTag> = new Set<BrowserMenuTag>([
"Skills",
"Status",
"Tasks"
])
]

const projectMenuTags: ReadonlySet<BrowserMenuTag> = new Set<BrowserMenuTag>(browserProjectMenuTags)

export const isProjectMenu = (menu: BrowserMenuTag): menu is BrowserProjectMenuTag => projectMenuTags.has(menu)

Expand Down
Loading
Loading