From 8355562b57d39d0bb9487bcf5be1073e18c962c1 Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 10:23:09 -0800 Subject: [PATCH 1/7] Fix electron preview in linux/wsl2 (#635) Fixes `pnpm run start` for Linux/WSL2 for shell. Should fix the playwright shell test as well. - Need to use `WebContent.loadFile` instead of `WebContent.loadUrl` API. - Ubuntu 24.04 needs `--no-sandbox` - Make sure we catch any error from loading those files instead of just hanging. - Fix the smoke test pipeline to clean up the .env file after everything is done (instead of right after the shell test) --- .github/workflows/shell-tests.yml | 26 ++++++++--------- .github/workflows/smoke-tests.yml | 33 ++++++++-------------- ts/packages/shell/src/main/index.ts | 23 +++++++++++---- ts/packages/shell/test/simple.spec.ts | 5 ++-- ts/packages/shell/test/testHelper.ts | 40 ++++++++++++++++----------- 5 files changed, 68 insertions(+), 59 deletions(-) diff --git a/.github/workflows/shell-tests.yml b/.github/workflows/shell-tests.yml index c881ffe1c..e0ce15004 100644 --- a/.github/workflows/shell-tests.yml +++ b/.github/workflows/shell-tests.yml @@ -40,9 +40,9 @@ jobs: - name: Setup Git LF run: | git config --global core.autocrlf false - + - uses: actions/checkout@v4 - + - uses: dorny/paths-filter@v3 id: filter with: @@ -50,49 +50,49 @@ jobs: ts: - "ts/**" - ".github/workflows/build-ts.yml" - + - uses: pnpm/action-setup@v4 name: Install pnpm with: package_json_file: ts/package.json - + - uses: actions/setup-node@v4 with: node-version: ${{ matrix.version }} cache: "pnpm" cache-dependency-path: ts/pnpm-lock.yaml - + - name: Install dependencies (pnpm) working-directory: ts run: | pnpm install --frozen-lockfile --strict-peer-dependencies - + - name: Install Playwright Browsers run: pnpm exec playwright install --with-deps working-directory: ts/packages/shell - + - name: Build repo working-directory: ts run: | npm run build - + - name: Login to Azure uses: azure/login@v2.2.0 with: client-id: ${{ secrets.AZUREAPPSERVICE_CLIENTID_5B0D2D6BA40F4710B45721D2112356DD }} tenant-id: ${{ secrets.AZUREAPPSERVICE_TENANTID_39BB903136F14B6EAD8F53A8AB78E3AA }} subscription-id: ${{ secrets.AZUREAPPSERVICE_SUBSCRIPTIONID_F36C1F2C4B2C49CA8DD5C52FAB98FA30 }} - + - name: Get Keys run: | node tools/scripts/getKeys.mjs --vault build-pipeline-kv working-directory: ts - + - name: Test CLI - verify .env & endpoint connectivity run: | npm run start:dev 'prompt' 'why is the sky blue' working-directory: ts/packages/cli - + - name: Shell Tests (windows) if: ${{ runner.os == 'windows' }} timeout-minutes: 60 @@ -110,10 +110,10 @@ jobs: sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 Xvfb :99 -screen 0 1600x1200x24 & export DISPLAY=:99 npm run shell:test - rm ../../.env + rm ../../.env working-directory: ts/packages/shell continue-on-error: true - + - uses: actions/upload-artifact@v4 if: ${{ !cancelled() }} with: diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 340d3e26d..d1317690c 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -12,7 +12,7 @@ on: # pull_request: # branches: ["main"] pull_request_target: - branches: ["main"] + branches: ["main"] merge_group: branches: ["main"] @@ -31,7 +31,7 @@ env: jobs: shell_and_cli: - #environment: ${{ github.event_name == 'pull_request_target' && 'development-fork' || 'development' }} # required for federated credentials + #environment: ${{ github.event_name == 'pull_request_target' && 'development-fork' || 'development' }} # required for federated credentials environment: development-fork strategy: fail-fast: false @@ -42,7 +42,7 @@ jobs: runs-on: ${{ matrix.os }} steps: - - if: runner.os == 'Linux' + - if: runner.os == 'Linux' run: | sudo apt install libsecret-1-0 @@ -54,9 +54,9 @@ jobs: uses: actions/checkout@v4 - if: ${{ github.event_name == 'pull_request_target'}} - uses: actions/checkout@v4 + uses: actions/checkout@v4 with: - ref: ${{ github.event.pull_request.head.sha }} + ref: ${{ github.event.pull_request.head.sha }} - uses: dorny/paths-filter@v3 id: filter @@ -114,37 +114,28 @@ jobs: timeout-minutes: 60 run: | npm run shell:smoke - rm ../../.env working-directory: ts/packages/shell continue-on-error: true - name: Shell Tests - smoke (linux) if: ${{ runner.os == 'Linux' }} timeout-minutes: 60 - # https://github.com/microsoft/playwright/issues/34251 - sysctl command run: | - sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 Xvfb :99 -screen 0 1600x1200x24 & export DISPLAY=:99 npm run shell:smoke - rm ../../.env working-directory: ts/packages/shell continue-on-error: true - # - name: Shell Tests - smoke (linux) - # if: ${{ runner.os == 'Linux' }} - # timeout-minutes: 60 - # # https://github.com/microsoft/playwright/issues/34251 - sysctl command - # run: | - # sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 - # xvfb-run --auto-servernum --server-args="-screen 0 16001200x24" -- npm run shell:smoke - # rm ../../.env - # working-directory: ts/packages/shell - # continue-on-error: true - - name: Live Tests if: ${{ runner.os == 'linux' }} timeout-minutes: 60 run: | npm run test:live working-directory: ts - continue-on-error: true \ No newline at end of file + continue-on-error: true + + - name: Clean up Keys + run: | + rm ./.env + working-directory: ts + if: always() diff --git a/ts/packages/shell/src/main/index.ts b/ts/packages/shell/src/main/index.ts index 4a2ceeec0..f5e18f48d 100644 --- a/ts/packages/shell/src/main/index.ts +++ b/ts/packages/shell/src/main/index.ts @@ -200,16 +200,25 @@ async function createWindow() { mainWindow.on("resize", setContentSize); + const contentLoadP: Promise[] = []; // HMR for renderer base on electron-vite cli. // Load the remote URL for development or the local html file for production. if (is.dev && process.env["ELECTRON_RENDERER_URL"]) { - chatView.webContents.loadURL(process.env["ELECTRON_RENDERER_URL"]); + contentLoadP.push( + chatView.webContents.loadURL(process.env["ELECTRON_RENDERER_URL"]), + ); } else { - chatView.webContents.loadURL(join(__dirname, "../renderer/index.html")); + contentLoadP.push( + chatView.webContents.loadFile( + join(__dirname, "../renderer/index.html"), + ), + ); } - mainWindow.webContents.loadURL( - join(__dirname, "../renderer/viewHost.html"), + contentLoadP.push( + mainWindow.webContents.loadFile( + join(__dirname, "../renderer/viewHost.html"), + ), ); mainWindow.removeMenu(); @@ -340,7 +349,7 @@ async function createWindow() { }; }); - return { mainWindow, chatView }; + return { mainWindow, chatView, contentLoadP }; } /** @@ -576,7 +585,7 @@ async function initialize() { await initializeSpeech(); - const { mainWindow, chatView } = await createWindow(); + const { mainWindow, chatView, contentLoadP } = await createWindow(); let settingSummary: string = ""; function updateSummary(dispatcher: Dispatcher) { @@ -599,6 +608,7 @@ async function initialize() { const dispatcherP = initializeDispatcher(chatView, updateSummary); ipcMain.on("dom ready", async () => { + debugShell("Showing window", performance.now() - time); mainWindow.show(); // Send settings asap @@ -683,6 +693,7 @@ async function initialize() { server.listen(pipePath); } + return Promise.all(contentLoadP); } app.whenReady() diff --git a/ts/packages/shell/test/simple.spec.ts b/ts/packages/shell/test/simple.spec.ts index 4ae374909..f48745e29 100644 --- a/ts/packages/shell/test/simple.spec.ts +++ b/ts/packages/shell/test/simple.spec.ts @@ -10,7 +10,7 @@ import test, { } from "@playwright/test"; import { exitApplication, - getAppPath, + getLaunchArgs, sendUserRequestAndWaitForResponse, startShell, } from "./testHelper"; @@ -21,9 +21,8 @@ test("dummy", async () => { test("simple", { tag: "@smoke" }, async ({}, testInfo) => { console.log(`Running test '${testInfo.title}`); - const app: ElectronApplication = await electron.launch({ - args: [getAppPath()], + args: getLaunchArgs(), }); const mainWindow: Page = await app.firstWindow(); await mainWindow.bringToFront(); diff --git a/ts/packages/shell/test/testHelper.ts b/ts/packages/shell/test/testHelper.ts index 727340d45..2775f8c6a 100644 --- a/ts/packages/shell/test/testHelper.ts +++ b/ts/packages/shell/test/testHelper.ts @@ -13,6 +13,7 @@ import { profile } from "node:console"; import fs from "node:fs"; import path from "node:path"; import os from "node:os"; +import { fileURLToPath } from "node:url"; const runningApplications: Map = new Map< string, @@ -27,7 +28,7 @@ export async function startShell(): Promise { process.env["INSTANCE_NAME"] = `test_${process.env["TEST_WORKER_INDEX"]}_${process.env["TEST_PARALLEL_INDEX"]}`; - // other related multi-instance varibles that need to be modfied to ensure we can run multiple shell instances + // other related multi-instance variables that need to be modified to ensure we can run multiple shell instances process.env["PORT"] = ( 9001 + parseInt(process.env["TEST_WORKER_INDEX"]!) ).toString(); @@ -50,7 +51,7 @@ export async function startShell(): Promise { `Starting electron instance '${process.env["INSTANCE_NAME"]}'`, ); const app: ElectronApplication = await electron.launch({ - args: [getAppPath()], + args: getLaunchArgs(), }); runningApplications.set(process.env["INSTANCE_NAME"]!, app); @@ -63,7 +64,7 @@ export async function startShell(): Promise { return mainWindow; } catch (e) { console.warn( - `Unable to start electrom application (${process.env["INSTANCE_NAME"]}). Attempt ${retryAttempt} of ${maxRetries}. Error: ${e}`, + `Unable to start electron application (${process.env["INSTANCE_NAME"]}). Attempt ${retryAttempt} of ${maxRetries}. Error: ${e}`, ); retryAttempt++; @@ -79,7 +80,7 @@ export async function startShell(): Promise { } while (retryAttempt <= maxRetries); throw new Error( - `Failed to start electrom app after ${maxRetries} attemps.`, + `Failed to start electron app after ${maxRetries} attempts.`, ); } @@ -132,21 +133,28 @@ export async function exitApplication(page: Page): Promise { } /** - * Gets the correct path based on test context (cmdline vs. VSCode extension) + * Gets the shell package path. * @returns The root path to the project containing the playwright configuration */ -export function getAppPath(): string { - if (fs.existsSync("playwright.config.ts")) { - return "."; - } else { - return path.join(".", "packages/shell"); - } +function getAppPath(): string { + return fileURLToPath(new URL("..", import.meta.url)); +} + +/** + * Get electron launch arguments + * @returns The arguments to pass to the electron application + */ +export function getLaunchArgs(): string[] { + const appPath = getAppPath(); + console.log(appPath); + // Ubuntu 24.04+ needs --no-sandbox, see https://github.com/electron/electron/issues/18265 + return os.platform() === "linux" ? [appPath, "--no-sandbox"] : [appPath]; } /** * Submits a user request to the system via the chat input box. * @param prompt The user request/prompt. - * @param page The maing page from the electron host application. + * @param page The main page from the electron host application. */ export async function sendUserRequest(prompt: string, page: Page) { const locator: Locator = page.locator("#phraseDiv"); @@ -159,7 +167,7 @@ export async function sendUserRequest(prompt: string, page: Page) { /** * Submits a user request to the system via the chat input box without waiting. * @param prompt The user request/prompt. - * @param page The maing page from the electron host application. + * @param page The main page from the electron host application. */ export async function sendUserRequestFast(prompt: string, page: Page) { const locator: Locator = page.locator("#phraseDiv"); @@ -171,7 +179,7 @@ export async function sendUserRequestFast(prompt: string, page: Page) { /** * Submits a user request to the system via the chat input box and then waits for the agent's response * @param prompt The user request/prompt. - * @param page The maing page from the electron host application. + * @param page The main page from the electron host application. */ export async function sendUserRequestAndWaitForResponse( prompt: string, @@ -193,7 +201,7 @@ export async function sendUserRequestAndWaitForResponse( /** * Gets the last agent message from the chat view - * @param page The maing page from the electron host application. + * @param page The main page from the electron host application. */ export async function getLastAgentMessage(page: Page): Promise { const locators: Locator[] = await page @@ -208,7 +216,7 @@ export async function getLastAgentMessage(page: Page): Promise { * @param page The page where the chatview is hosted * @param timeout The maximum amount of time to wait for the agent message * @param expectedMessageCount The expected # of agent messages at this time. - * @returns When the expected # of messages is reached or the timeout is reached. Whichever occurrs first. + * @returns When the expected # of messages is reached or the timeout is reached. Whichever occurs first. */ export async function waitForAgentMessage( page: Page, From bba9a3203a6bb6b226a393c132108384b7f718cf Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 10:27:40 -0800 Subject: [PATCH 2/7] Disable pull-request-target for now (#636) --- .github/workflows/build-ts-pull-request-target.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build-ts-pull-request-target.yml b/.github/workflows/build-ts-pull-request-target.yml index dd2680d62..daf3efccd 100644 --- a/.github/workflows/build-ts-pull-request-target.yml +++ b/.github/workflows/build-ts-pull-request-target.yml @@ -7,9 +7,9 @@ name: build-ts on: # Manual approval of forked environments is required - pull_request_target: - branches: - - main + # pull_request_target: + # branches: + # - main concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} @@ -113,7 +113,7 @@ jobs: run: | Xvfb :99 -screen 0 1600x1200x24 & export DISPLAY=:99 npx playwright test simple.spec.ts - rm ../../.env + rm ../../.env working-directory: ts/packages/shell continue-on-error: true - name: Live Tests @@ -122,4 +122,4 @@ jobs: run: | npm run test:live working-directory: ts - continue-on-error: true \ No newline at end of file + continue-on-error: true From a8eb7ef4f2b538cf25d9fa6af3da9930a4afd9e8 Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 10:59:01 -0800 Subject: [PATCH 3/7] Fix azure pipeline (#638) There is no need to install an older version of pnpm again after we use corepack to install it. --- pipelines/azure-build-ts.yml | 157 ++++++++++++++--------------- pipelines/include-install-pnpm.yml | 76 +++++++------- 2 files changed, 112 insertions(+), 121 deletions(-) diff --git a/pipelines/azure-build-ts.yml b/pipelines/azure-build-ts.yml index f4983cb0a..4b641b4c2 100644 --- a/pipelines/azure-build-ts.yml +++ b/pipelines/azure-build-ts.yml @@ -20,89 +20,80 @@ pr: - "*" pool: - vmImage: 'ubuntu-latest' + vmImage: "ubuntu-latest" variables: - node_version: '18' - pnpm_version: '9.0.0' - workingDirectory: 'ts' - agentsdk_folder: 'ts/packages/agentSdk' - + workingDirectory: "ts" + agentsdk_folder: "ts/packages/agentSdk" + jobs: -- job: build_ts - displayName: 'Build TypeScript Project' - strategy: - matrix: - node_20: - nodeVersion: '20' - steps: - - checkout: TypeAgent-GH - displayName: 'Checkout TypeAgent Repository' - path: 'typeagent' - - - template: include-install-pnpm.yml - parameters: - buildDirectory: $(Build.SourcesDirectory)/ts - - - script: | - curl -fsSL https://get.pnpm.io/v6.14.js | node - add --global pnpm@$(pnpm_version) - export PNPM_HOME="$HOME/.local/share/pnpm" - export PATH="$PNPM_HOME:$PATH" - displayName: 'Install pnpm' - condition: always() - - - task: UseNode@1 - displayName: 'Setup Node.js' - inputs: - version: $(nodeVersion) - checkLatest: true - - - script: | - pnpm install --frozen-lockfile --strict-peer-dependencies - displayName: 'Install dependencies' - workingDirectory: $(workingDirectory) - - - script: | - current_version=$(node -p "require('./package.json').version") - new_version="${current_version}-$(Build.BuildId)" - echo "##vso[task.setvariable variable=package_version]$new_version" - jq ".version=\"$new_version\"" package.json > package.tmp.json - mv package.tmp.json package.json - echo "Updated package.json to version $new_version" - cat package.json - displayName: 'Update Package Version' - workingDirectory: $(agentsdk_folder) - - - script: | - npm run build - displayName: 'Build' - workingDirectory: $(workingDirectory) - - - script: | - npm run test - displayName: 'Run Tests' - workingDirectory: $(workingDirectory) - - - script: | - npm run lint - displayName: 'Lint' - workingDirectory: $(workingDirectory) - - - script: | - echo $(ADO_REGISTRY) - echo "registry=$(ADO_REGISTRY)" > .npmrc - echo "always-auth=true" >> .npmrc - cat .npmrc - displayName: 'Create .npmrc file.' - workingDirectory: $(agentsdk_folder) - - - task: npmAuthenticate@0 - inputs: - workingFile: '$(agentsdk_folder)/.npmrc' - displayName: 'Authenticate with Azure Artifacts' - - - script: | - cd $(Build.SourcesDirectory)/$(agentsdk_folder) - npm publish --registry=$(ADO_REGISTRY) - displayName: 'Pack and Publish agent-sdk Module' - condition: succeeded() + - job: build_ts + displayName: "Build TypeScript Project" + strategy: + matrix: + node_20: + nodeVersion: "22" + steps: + - checkout: TypeAgent-GH + displayName: "Checkout TypeAgent Repository" + path: "typeagent" + + - template: include-install-pnpm.yml + parameters: + buildDirectory: $(Build.SourcesDirectory)/ts + + - task: UseNode@1 + displayName: "Setup Node.js" + inputs: + version: $(nodeVersion) + checkLatest: true + + - script: | + pnpm install --frozen-lockfile --strict-peer-dependencies + displayName: "Install dependencies" + workingDirectory: $(workingDirectory) + + - script: | + current_version=$(node -p "require('./package.json').version") + new_version="${current_version}-$(Build.BuildId)" + echo "##vso[task.setvariable variable=package_version]$new_version" + jq ".version=\"$new_version\"" package.json > package.tmp.json + mv package.tmp.json package.json + echo "Updated package.json to version $new_version" + cat package.json + displayName: "Update Package Version" + workingDirectory: $(agentsdk_folder) + + - script: | + npm run build + displayName: "Build" + workingDirectory: $(workingDirectory) + + - script: | + npm run test + displayName: "Run Tests" + workingDirectory: $(workingDirectory) + + - script: | + npm run lint + displayName: "Lint" + workingDirectory: $(workingDirectory) + + - script: | + echo $(ADO_REGISTRY) + echo "registry=$(ADO_REGISTRY)" > .npmrc + echo "always-auth=true" >> .npmrc + cat .npmrc + displayName: "Create .npmrc file." + workingDirectory: $(agentsdk_folder) + + - task: npmAuthenticate@0 + inputs: + workingFile: "$(agentsdk_folder)/.npmrc" + displayName: "Authenticate with Azure Artifacts" + + - script: | + cd $(Build.SourcesDirectory)/$(agentsdk_folder) + npm publish --registry=$(ADO_REGISTRY) + displayName: "Pack and Publish agent-sdk Module" + condition: succeeded() diff --git a/pipelines/include-install-pnpm.yml b/pipelines/include-install-pnpm.yml index a7a94c48c..cc90d4123 100644 --- a/pipelines/include-install-pnpm.yml +++ b/pipelines/include-install-pnpm.yml @@ -6,46 +6,46 @@ # This template can be included in pipelines to install pnpm with store caching enabled. parameters: -# The path containing the project(s) to build. -- name: buildDirectory - type: string + # The path containing the project(s) to build. + - name: buildDirectory + type: string -# If set to false, the pnpm store will not be cached or restored from cache. -- name: enableCache - type: boolean - default: true + # If set to false, the pnpm store will not be cached or restored from cache. + - name: enableCache + type: boolean + default: true -# The path to the pnpm store. The contents here will be cached and restored when using pnpm in a pipeline. -- name: pnpmStorePath - type: string - default: $(Pipeline.Workspace)/.pnpm-store + # The path to the pnpm store. The contents here will be cached and restored when using pnpm in a pipeline. + - name: pnpmStorePath + type: string + default: $(Pipeline.Workspace)/.pnpm-store steps: -- ${{ if eq(parameters.enableCache, true) }}: - - task: Cache@2 - displayName: Cache pnpm store - timeoutInMinutes: 3 - continueOnError: true - inputs: - # Caches are already scoped to individual pipelines, so no need to include the release group name or tag - # in the cache key - key: 'pnpm-store | "$(Agent.OS)" | ${{ parameters.buildDirectory }}/pnpm-lock.yaml' - path: ${{ parameters.pnpmStorePath }} - restoreKeys: | - pnpm-store | "$(Agent.OS)" + - ${{ if eq(parameters.enableCache, true) }}: + - task: Cache@2 + displayName: Cache pnpm store + timeoutInMinutes: 3 + continueOnError: true + inputs: + # Caches are already scoped to individual pipelines, so no need to include the release group name or tag + # in the cache key + key: 'pnpm-store | "$(Agent.OS)" | ${{ parameters.buildDirectory }}/pnpm-lock.yaml' + path: ${{ parameters.pnpmStorePath }} + restoreKeys: | + pnpm-store | "$(Agent.OS)" -- task: Bash@3 - displayName: Install and configure pnpm - inputs: - targetType: 'inline' - workingDirectory: ${{ parameters.buildDirectory }} - # The previous task (cache restoration) can timeout, which is classified as canceled, but since it's just cache - # restoration, we want to continue even if it timed out. - condition: or(succeeded(), canceled()) - # workspace-concurrency 0 means use use the CPU core count. This is better than the default (4) for larger agents. - script: | - echo "Using node $(node --version)" - sudo corepack enable - echo "Using pnpm $(pnpm -v)" - pnpm config set store-dir ${{ parameters.pnpmStorePath }} - pnpm config set -g workspace-concurrency 0 \ No newline at end of file + - task: Bash@3 + displayName: Install and configure pnpm + inputs: + targetType: "inline" + workingDirectory: ${{ parameters.buildDirectory }} + # The previous task (cache restoration) can timeout, which is classified as canceled, but since it's just cache + # restoration, we want to continue even if it timed out. + condition: or(succeeded(), canceled()) + # workspace-concurrency 0 means use use the CPU core count. This is better than the default (4) for larger agents. + script: | + echo "Using node $(node --version)" + sudo corepack enable + echo "Using pnpm $(pnpm -v)" + pnpm config set store-dir ${{ parameters.pnpmStorePath }} + pnpm config set -g workspace-concurrency 0 From 8b61424b6300ff2da05fa3b0de9d8c02967c122e Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 11:04:42 -0800 Subject: [PATCH 4/7] local --- pipelines/azure-build-ts.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pipelines/azure-build-ts.yml b/pipelines/azure-build-ts.yml index 4b641b4c2..267f55d36 100644 --- a/pipelines/azure-build-ts.yml +++ b/pipelines/azure-build-ts.yml @@ -31,7 +31,7 @@ jobs: displayName: "Build TypeScript Project" strategy: matrix: - node_20: + node_22: nodeVersion: "22" steps: - checkout: TypeAgent-GH @@ -70,8 +70,8 @@ jobs: workingDirectory: $(workingDirectory) - script: | - npm run test - displayName: "Run Tests" + npm run test:local + displayName: "Run Tests (Local)" workingDirectory: $(workingDirectory) - script: | From 24863ceed030473596e00c65e9f879c8f88f9cbe Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 12:48:11 -0800 Subject: [PATCH 5/7] Switch to use `WebContentsView` from deprecated `BrowserView` (#637) --- ts/packages/shell/src/main/index.ts | 62 ++++++++++++++-------------- ts/packages/shell/test/testHelper.ts | 2 +- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/ts/packages/shell/src/main/index.ts b/ts/packages/shell/src/main/index.ts index f5e18f48d..efe97bc9d 100644 --- a/ts/packages/shell/src/main/index.ts +++ b/ts/packages/shell/src/main/index.ts @@ -11,8 +11,8 @@ import { dialog, DevicePermissionHandlerHandlerDetails, WebContents, - BrowserView, session, + WebContentsView, } from "electron"; import path, { join } from "node:path"; import { electronApp, optimizer, is } from "@electron-toolkit/utils"; @@ -60,8 +60,8 @@ process.argv.forEach((arg) => { }); let mainWindow: BrowserWindow | null = null; -let inlineBrowserView: BrowserView | null = null; -let chatView: BrowserView | null = null; +let inlineWebContentView: WebContentsView | null = null; +let chatView: WebContentsView | null = null; const inlineBrowserSize = 1000; const userAgent = @@ -74,9 +74,9 @@ function setContentSize() { let newWidth = newBounds.width; let chatWidth = chatView.getBounds().width; - if (inlineBrowserView) { + if (inlineWebContentView) { let browserWidth = newWidth - chatWidth; - inlineBrowserView?.setBounds({ + inlineWebContentView?.setBounds({ x: chatWidth + 4, y: 0, width: browserWidth, @@ -140,7 +140,7 @@ async function createWindow() { allowFileAccess: true, }); - chatView = new BrowserView({ + chatView = new WebContentsView({ webPreferences: { preload: join(__dirname, "../preload/index.mjs"), sandbox: false, @@ -150,7 +150,7 @@ async function createWindow() { chatView.webContents.setUserAgent(userAgent); - // ensure links are openend in a new browser window + // ensure links are opened in a new browser window chatView.webContents.setWindowOpenHandler((details) => { // TODO: add logic for keeping things in the browser window shell.openExternal(details.url); @@ -159,7 +159,7 @@ async function createWindow() { setContentSize(); - mainWindow.setBrowserView(chatView); + mainWindow.contentView.addChildView(chatView); setupDevicePermissions(mainWindow); @@ -239,8 +239,8 @@ async function createWindow() { height: chatBounds?.height!, }); - if (inlineBrowserView) { - inlineBrowserView?.setBounds({ + if (inlineWebContentView) { + inlineWebContentView?.setBounds({ x: chatBounds!.width + 4, y: 0, width: bounds.width - newWidth - 20, @@ -295,20 +295,20 @@ async function createWindow() { ): void => { const mainWindowSize = mainWindow?.getBounds(); - if (!inlineBrowserView && mainWindowSize) { - inlineBrowserView = new BrowserView({ + if (!inlineWebContentView && mainWindowSize) { + inlineWebContentView = new WebContentsView({ webPreferences: { preload: join(__dirname, "../preload-cjs/webview.cjs"), sandbox: false, }, }); - inlineBrowserView.webContents.setUserAgent(userAgent); + inlineWebContentView.webContents.setUserAgent(userAgent); - mainWindow?.addBrowserView(inlineBrowserView); + mainWindow?.contentView.addChildView(inlineWebContentView); - setupDevToolsHandlers(inlineBrowserView); - setupZoomHandlers(inlineBrowserView); + setupDevToolsHandlers(inlineWebContentView); + setupZoomHandlers(inlineWebContentView); mainWindow?.setBounds({ width: mainWindowSize.width + inlineBrowserSize, @@ -316,17 +316,17 @@ async function createWindow() { setContentSize(); } - inlineBrowserView?.webContents.loadURL(targetUrl.toString()); + inlineWebContentView?.webContents.loadURL(targetUrl.toString()); }; ShellSettings.getinstance().onCloseInlineBrowser = (): void => { const mainWindowSize = mainWindow?.getBounds(); - if (inlineBrowserView && mainWindowSize) { - const browserBounds = inlineBrowserView.getBounds(); - inlineBrowserView.webContents.close(); - mainWindow?.removeBrowserView(inlineBrowserView); - inlineBrowserView = null; + if (inlineWebContentView && mainWindowSize) { + const browserBounds = inlineWebContentView.getBounds(); + inlineWebContentView.webContents.close(); + mainWindow?.contentView.removeChildView(inlineWebContentView); + inlineWebContentView = null; mainWindow?.setBounds({ width: mainWindowSize.width - browserBounds.width, @@ -342,7 +342,7 @@ async function createWindow() { BrowserAgentIpc.getinstance().onMessageReceived = ( message: WebSocketMessageV2, ) => { - inlineBrowserView?.webContents.send( + inlineWebContentView?.webContents.send( "received-from-browser-ipc", message, ); @@ -503,7 +503,7 @@ async function initializeSpeech() { } async function initializeDispatcher( - chatView: BrowserView, + chatView: WebContentsView, updateSummary: (dispatcher: Dispatcher) => void, ) { const clientIOChannel = createGenericChannel((message: any) => { @@ -752,15 +752,15 @@ app.on("window-all-closed", () => { } }); -function zoomIn(chatView: BrowserView) { +function zoomIn(chatView: WebContentsView) { setZoomLevel(chatView.webContents.zoomFactor + 0.1, chatView); } -function zoomOut(chatView: BrowserView) { +function zoomOut(chatView: WebContentsView) { setZoomLevel(chatView.webContents.zoomFactor - 0.1, chatView); } -function setZoomLevel(zoomLevel: number, chatView: BrowserView | null) { +function setZoomLevel(zoomLevel: number, chatView: WebContentsView | null) { if (zoomLevel < 0.1) { zoomLevel = 0.1; } else if (zoomLevel > 10) { @@ -773,11 +773,11 @@ function setZoomLevel(zoomLevel: number, chatView: BrowserView | null) { updateZoomInTitle(chatView!); } -function resetZoom(chatView: BrowserView) { +function resetZoom(chatView: WebContentsView) { setZoomLevel(1, chatView); } -function updateZoomInTitle(chatView: BrowserView) { +function updateZoomInTitle(chatView: WebContentsView) { const prevTitle = mainWindow?.getTitle(); if (prevTitle) { let summary = prevTitle.substring(0, prevTitle.indexOf("Zoom: ")); @@ -789,7 +789,7 @@ function updateZoomInTitle(chatView: BrowserView) { } const isMac = process.platform === "darwin"; -function setupZoomHandlers(chatView: BrowserView) { +function setupZoomHandlers(chatView: WebContentsView) { chatView.webContents.on("before-input-event", (_event, input) => { if ((isMac ? input.meta : input.control) && input.type === "keyDown") { if ( @@ -816,7 +816,7 @@ function setupZoomHandlers(chatView: BrowserView) { }); } -function setupDevToolsHandlers(view: BrowserView) { +function setupDevToolsHandlers(view: WebContentsView) { view.webContents.on("before-input-event", (_event, input) => { if (input.type === "keyDown") { if (!is.dev) { diff --git a/ts/packages/shell/test/testHelper.ts b/ts/packages/shell/test/testHelper.ts index 2775f8c6a..c2b0d13a6 100644 --- a/ts/packages/shell/test/testHelper.ts +++ b/ts/packages/shell/test/testHelper.ts @@ -137,7 +137,7 @@ export async function exitApplication(page: Page): Promise { * @returns The root path to the project containing the playwright configuration */ function getAppPath(): string { - return fileURLToPath(new URL("..", import.meta.url)); + return new URL("..", import.meta.url).toString(); } /** From b7aacddd755d77417056a7d523844b2d19ffc107 Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 14:33:56 -0800 Subject: [PATCH 6/7] Fix electron app path (#639) - It doesn't like the trailing path separator. - Add check in simple smoke test to make sure we catch the load error in the future by checking the URL that is loaded. - Add smoke test to exercise `startShell` which the nightly shell test depends on. --- ts/packages/shell/test/simple.spec.ts | 12 +++++++----- ts/packages/shell/test/testHelper.ts | 13 +++++++------ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ts/packages/shell/test/simple.spec.ts b/ts/packages/shell/test/simple.spec.ts index f48745e29..575384496 100644 --- a/ts/packages/shell/test/simple.spec.ts +++ b/ts/packages/shell/test/simple.spec.ts @@ -10,25 +10,27 @@ import test, { } from "@playwright/test"; import { exitApplication, + getAppPath, getLaunchArgs, sendUserRequestAndWaitForResponse, startShell, } from "./testHelper"; - -test("dummy", async () => { - // do nothing -}); +import { fileURLToPath } from "node:url"; test("simple", { tag: "@smoke" }, async ({}, testInfo) => { - console.log(`Running test '${testInfo.title}`); const app: ElectronApplication = await electron.launch({ args: getLaunchArgs(), }); const mainWindow: Page = await app.firstWindow(); await mainWindow.bringToFront(); + expect(fileURLToPath(mainWindow.url())).toContain(getAppPath()); await app.close(); }); +test("startShell", { tag: "@smoke" }, async ({}) => { + await startShell(); +}); + test.skip("why is the sky blue?", { tag: "@smoke" }, async ({}, testInfo) => { console.log(`Running test '${testInfo.title}`); diff --git a/ts/packages/shell/test/testHelper.ts b/ts/packages/shell/test/testHelper.ts index c2b0d13a6..a61cf33f0 100644 --- a/ts/packages/shell/test/testHelper.ts +++ b/ts/packages/shell/test/testHelper.ts @@ -2,14 +2,11 @@ // Licensed under the MIT License. import { - _electron, _electron as electron, ElectronApplication, Locator, Page, - TestDetails, } from "@playwright/test"; -import { profile } from "node:console"; import fs from "node:fs"; import path from "node:path"; import os from "node:os"; @@ -136,8 +133,13 @@ export async function exitApplication(page: Page): Promise { * Gets the shell package path. * @returns The root path to the project containing the playwright configuration */ -function getAppPath(): string { - return new URL("..", import.meta.url).toString(); +export function getAppPath(): string { + const packagePath = fileURLToPath(new URL("..", import.meta.url)); + const appPath = packagePath.endsWith(path.sep) + ? packagePath.slice(0, -1) + : packagePath; + + return appPath; } /** @@ -146,7 +148,6 @@ function getAppPath(): string { */ export function getLaunchArgs(): string[] { const appPath = getAppPath(); - console.log(appPath); // Ubuntu 24.04+ needs --no-sandbox, see https://github.com/electron/electron/issues/18265 return os.platform() === "linux" ? [appPath, "--no-sandbox"] : [appPath]; } From 9dd2d10cdf6b370170493d74a6cb69c9bd28c194 Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Thu, 30 Jan 2025 14:40:33 -0800 Subject: [PATCH 7/7] Fix action command handler with no parameters (#640) Also fix the test script that should call test:local in package.json --- ts/examples/memoryProviders/package.json | 2 +- ts/packages/actionSchema/package.json | 2 +- ts/packages/actionSchema/src/validate.ts | 2 +- ts/packages/agents/test/src/handler.ts | 4 +++- ts/packages/agents/test/src/schema.ts | 6 +++++- ts/packages/aiclient/package.json | 2 +- ts/packages/api/package.json | 2 +- ts/packages/cache/package.json | 2 +- ts/packages/commonUtils/package.json | 2 +- ts/packages/dispatcher/package.json | 2 +- ts/packages/dispatcher/src/context/system/systemAgent.ts | 5 ++++- ts/packages/dispatcher/test/provider.spec.ts | 6 ++++++ ts/packages/knowledgeProcessor/package.json | 2 +- ts/packages/telemetry/package.json | 2 +- ts/packages/typeagent/package.json | 2 +- 15 files changed, 29 insertions(+), 14 deletions(-) diff --git a/ts/examples/memoryProviders/package.json b/ts/examples/memoryProviders/package.json index 1003aed51..f674572f6 100644 --- a/ts/examples/memoryProviders/package.json +++ b/ts/examples/memoryProviders/package.json @@ -21,7 +21,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/actionSchema/package.json b/ts/packages/actionSchema/package.json index 4e69bae13..b8a95852f 100644 --- a/ts/packages/actionSchema/package.json +++ b/ts/packages/actionSchema/package.json @@ -23,7 +23,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/actionSchema/src/validate.ts b/ts/packages/actionSchema/src/validate.ts index 8a3689279..4095268f0 100644 --- a/ts/packages/actionSchema/src/validate.ts +++ b/ts/packages/actionSchema/src/validate.ts @@ -147,7 +147,7 @@ function validateObject( ignoreExtraneous?.includes(actualField) !== true ) { const fullName = name ? `${name}.${actualField}` : actualField; - throw new Error(`Extraneous property ${fullName}`); + throw new Error(`Extraneous property '${fullName}'`); } } } diff --git a/ts/packages/agents/test/src/handler.ts b/ts/packages/agents/test/src/handler.ts index b6922374f..0f22fe2fc 100644 --- a/ts/packages/agents/test/src/handler.ts +++ b/ts/packages/agents/test/src/handler.ts @@ -52,7 +52,9 @@ async function executeAction( case "add": const { a, b } = action.parameters; return createActionResult(`The sum of ${a} and ${b} is ${a + b}`); + case "random": + return createActionResult(`Random number: ${Math.random()}`); default: - throw new Error(`Unknown action: ${action.actionName}`); + throw new Error(`Unknown action: ${(action as any).actionName}`); } } diff --git a/ts/packages/agents/test/src/schema.ts b/ts/packages/agents/test/src/schema.ts index 5334936f9..43e03250e 100644 --- a/ts/packages/agents/test/src/schema.ts +++ b/ts/packages/agents/test/src/schema.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -export type TestActions = AddAction; +export type TestActions = AddAction | RandomNumberAction; type AddAction = { actionName: "add"; parameters: { @@ -9,3 +9,7 @@ type AddAction = { b: number; }; }; + +type RandomNumberAction = { + actionName: "random"; +}; diff --git a/ts/packages/aiclient/package.json b/ts/packages/aiclient/package.json index 89b631e15..6a05955e7 100644 --- a/ts/packages/aiclient/package.json +++ b/ts/packages/aiclient/package.json @@ -22,7 +22,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/api/package.json b/ts/packages/api/package.json index 13ba834cf..a373ca2bb 100644 --- a/ts/packages/api/package.json +++ b/ts/packages/api/package.json @@ -18,7 +18,7 @@ "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", "start": "node ./dist/index.js", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/cache/package.json b/ts/packages/cache/package.json index c23c6673e..12b016b68 100644 --- a/ts/packages/cache/package.json +++ b/ts/packages/cache/package.json @@ -22,7 +22,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/commonUtils/package.json b/ts/packages/commonUtils/package.json index 3247505c3..c9ad4cc7a 100644 --- a/ts/packages/commonUtils/package.json +++ b/ts/packages/commonUtils/package.json @@ -23,7 +23,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/dispatcher/package.json b/ts/packages/dispatcher/package.json index fd2c088b8..00c6c0aa7 100644 --- a/ts/packages/dispatcher/package.json +++ b/ts/packages/dispatcher/package.json @@ -27,7 +27,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/dispatcher/src/context/system/systemAgent.ts b/ts/packages/dispatcher/src/context/system/systemAgent.ts index 9bacc615c..65d171c2e 100644 --- a/ts/packages/dispatcher/src/context/system/systemAgent.ts +++ b/ts/packages/dispatcher/src/context/system/systemAgent.ts @@ -226,9 +226,12 @@ class ActionCommandHandler implements CommandHandler { const action: AppAction = { translatorName, actionName, - parameters: params.flags.parameters, }; + if (params.flags.parameters !== undefined) { + action.parameters = params.flags.parameters; + } + validateAction(actionSchema, action, true); return executeActions( diff --git a/ts/packages/dispatcher/test/provider.spec.ts b/ts/packages/dispatcher/test/provider.spec.ts index d57880581..13aeb8c5e 100644 --- a/ts/packages/dispatcher/test/provider.spec.ts +++ b/ts/packages/dispatcher/test/provider.spec.ts @@ -60,6 +60,12 @@ describe("dispatcher", () => { expect(output[1].message).toBe("The sum of 1 and 2 is 3"); }); + it("action command no parameters", async () => { + await dispatcher.processCommand("@action test random"); + + expect(output).toHaveLength(2); + expect(output[1].message).toMatch(/Random number: [0-9.]+/); + }); const errorCommands = [ { name: "Empty Command", diff --git a/ts/packages/knowledgeProcessor/package.json b/ts/packages/knowledgeProcessor/package.json index e4052e50c..9e5bc25b4 100644 --- a/ts/packages/knowledgeProcessor/package.json +++ b/ts/packages/knowledgeProcessor/package.json @@ -21,7 +21,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/telemetry/package.json b/ts/packages/telemetry/package.json index 7a13dd891..3e3abdd9c 100644 --- a/ts/packages/telemetry/package.json +++ b/ts/packages/telemetry/package.json @@ -20,7 +20,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b" diff --git a/ts/packages/typeagent/package.json b/ts/packages/typeagent/package.json index 0ce0f1653..7c9f06c54 100644 --- a/ts/packages/typeagent/package.json +++ b/ts/packages/typeagent/package.json @@ -22,7 +22,7 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../prettierignore", - "test": "npm run test", + "test": "npm run test:local", "test:local": "node --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "test:local:debug": "node --inspect-brk --no-warnings --experimental-vm-modules ./node_modules/jest/bin/jest.js --testPathPattern=\".*\\.spec\\.js\"", "tsc": "tsc -b"