From f8d6dd6342b534e8f986a5c376efa90146bf3773 Mon Sep 17 00:00:00 2001 From: suraj_markup <96483690+suraj-markup@users.noreply.github.com> Date: Sat, 23 May 2026 15:40:18 +0530 Subject: [PATCH] Fix ao update workspace rebuild (#2033) * fix update workspace rebuild * Harden ao update script root detection --- .../cli/__tests__/commands/update.test.ts | 6 +- .../__tests__/scripts/update-script.test.ts | 44 ++++++++++++- packages/cli/src/assets/scripts/ao-update.ps1 | 40 +++++++++--- packages/cli/src/assets/scripts/ao-update.sh | 44 ++++++++++--- .../src/__tests__/version-update-api.test.ts | 65 ++++++++++++++++--- 5 files changed, 168 insertions(+), 31 deletions(-) diff --git a/packages/cli/__tests__/commands/update.test.ts b/packages/cli/__tests__/commands/update.test.ts index 843bcc1bb..a567dc22e 100644 --- a/packages/cli/__tests__/commands/update.test.ts +++ b/packages/cli/__tests__/commands/update.test.ts @@ -1002,7 +1002,7 @@ describe("update command", () => { } }); - it("actually invokes runNpmInstall when AO_NON_INTERACTIVE_INSTALL=1 even though isTTY is false", async () => { + it("invokes runNpmInstall exactly once when AO_NON_INTERACTIVE_INSTALL=1 even though isTTY is false", async () => { // The P1 bug: before this fix, the !isTTY() branch printed "Run: ..." // and returned. The dashboard's banner click would 202 but no install // would run. Asserting spawn was called proves the install actually @@ -1013,6 +1013,10 @@ describe("update command", () => { expect.arrayContaining(["install"]), expect.anything(), ); + const installCalls = mockSpawn.mock.calls.filter(([cmd, args]) => { + return cmd === "npm" && Array.isArray(args) && args.includes("install"); + }); + expect(installCalls).toHaveLength(1); // And without a TTY, we MUST NOT have prompted — that would hang the // detached child forever. expect(mockPromptConfirm).not.toHaveBeenCalled(); diff --git a/packages/cli/__tests__/scripts/update-script.test.ts b/packages/cli/__tests__/scripts/update-script.test.ts index dac068f40..03b6d2523 100644 --- a/packages/cli/__tests__/scripts/update-script.test.ts +++ b/packages/cli/__tests__/scripts/update-script.test.ts @@ -82,8 +82,10 @@ esac\nexit 0`, expect(commands).toContain("git fetch origin main"); expect(commands).toContain("git pull --ff-only origin main"); expect(commands).toContain("pnpm install"); - expect(commands).toContain("pnpm --filter @aoagents/ao-core clean"); - expect(commands).toContain("pnpm --filter @aoagents/ao-cli build"); + expect(commands).toContain("pnpm -r --if-present clean"); + expect(commands).toContain("pnpm build"); + expect(commands).not.toContain("pnpm --filter @aoagents/ao-core clean"); + expect(commands).not.toContain("pnpm --filter @aoagents/ao-cli build"); expect(commands).toContain("npm link --force"); }); @@ -267,6 +269,43 @@ exit 0`, ); }); + it.skipIf(process.platform === "win32")( + "resolves the source checkout root when AO_REPO_ROOT is unset", + () => { + const tempRoot = mkdtempSync(join(tmpdir(), "ao-update-root-detect-")); + const binDir = join(tempRoot, "bin"); + mkdirSync(binDir, { recursive: true }); + const commandLog = join(tempRoot, "commands.log"); + createFakeBinary( + binDir, + "node", + `if [ "$1" = "--version" ]; then printf 'v20.11.1\\n'; fi +printf 'node %s\\n' "$*" >> ${JSON.stringify(commandLog)} +exit 0`, + ); + + const env = { + ...process.env, + PATH: `${binDir}:${process.env.PATH || ""}`, + }; + delete env["AO_REPO_ROOT"]; + + const result = spawnSync("bash", [scriptPath, "--smoke-only"], { + env, + encoding: "utf8", + }); + + const commands = readFileSync(commandLog, "utf8"); + rmSync(tempRoot, { recursive: true, force: true }); + + const repoRoot = resolve(packageRoot, "../.."); + expect(result.status).toBe(0); + expect(commands).toContain( + `node ${join(repoRoot, "packages", "ao", "bin", "ao.js")} --version`, + ); + }, + ); + it("fails fast on a dirty install repo with an actionable message", () => { const tempRoot = mkdtempSync(join(tmpdir(), "ao-update-dirty-")); const fakeRepo = join(tempRoot, "repo"); @@ -374,6 +413,7 @@ exit 0`, expect(result.stdout).toContain("Already on latest version"); // Rebuild commands should NOT have run expect(commands).not.toContain("pnpm install"); + expect(commands).not.toContain("pnpm build"); expect(commands).not.toContain("pnpm --filter @aoagents/ao-core build"); expect(commands).not.toContain("npm link"); expect(commands).not.toContain("git pull --ff-only origin main"); diff --git a/packages/cli/src/assets/scripts/ao-update.ps1 b/packages/cli/src/assets/scripts/ao-update.ps1 index 4e1522e3d..99d52c8ce 100644 --- a/packages/cli/src/assets/scripts/ao-update.ps1 +++ b/packages/cli/src/assets/scripts/ao-update.ps1 @@ -27,7 +27,7 @@ if ($Help) { Usage: ao update [--skip-smoke] [--smoke-only] Fast-forwards the local Agent Orchestrator install repo to main, installs deps, -clean-rebuilds critical packages, refreshes the ao launcher, and runs smoke tests. +clean-rebuilds all workspace packages, refreshes the ao launcher, and runs smoke tests. Options: --skip-smoke Skip smoke tests after rebuild @@ -42,7 +42,34 @@ if ($SkipSmoke -and $SmokeOnly) { } $TargetBranch = if ($env:AO_UPDATE_BRANCH) { $env:AO_UPDATE_BRANCH } else { 'main' } -$RepoRoot = if ($env:AO_REPO_ROOT) { $env:AO_REPO_ROOT } else { (Resolve-Path (Join-Path $PSScriptRoot '..')).Path } + +function Test-AoRepoRoot([string]$path) { + return (Test-Path (Join-Path $path 'packages/ao/bin/ao.js')) -and + (Test-Path (Join-Path $path 'packages/cli')) +} + +function Find-RepoRootFrom([string]$start) { + $dir = (Resolve-Path $start).Path + while ($dir) { + if (Test-AoRepoRoot $dir) { return $dir } + $parent = Split-Path -Parent $dir + if (-not $parent -or $parent -eq $dir) { break } + $dir = $parent + } + return $null +} + +function Resolve-RepoRoot { + if ($env:AO_REPO_ROOT) { return $env:AO_REPO_ROOT } + $fromScript = Find-RepoRootFrom $PSScriptRoot + if ($fromScript) { return $fromScript } + $fromCwd = Find-RepoRootFrom (Get-Location).Path + if ($fromCwd) { return $fromCwd } + Write-Error "Unable to find Agent Orchestrator repo root. Fix: run via ao update or set AO_REPO_ROOT." + exit 1 +} + +$RepoRoot = Resolve-RepoRoot function Require-Command([string]$name, [string]$fixHint) { if (-not (Get-Command $name -ErrorAction SilentlyContinue)) { @@ -170,13 +197,8 @@ if (-not $SmokeOnly) { Run-Cmd git pull --ff-only $UpdateRemote $TargetBranch Run-Cmd pnpm install - Run-Cmd pnpm --filter @aoagents/ao-core clean - Run-Cmd pnpm --filter @aoagents/ao-cli clean - Run-Cmd pnpm --filter @aoagents/ao-web clean - - Run-Cmd pnpm --filter @aoagents/ao-core build - Run-Cmd pnpm --filter @aoagents/ao-cli build - Run-Cmd pnpm --filter @aoagents/ao-web build + Run-Cmd pnpm -r --if-present clean + Run-Cmd pnpm build Write-Host "" Write-Host "Refreshing ao launcher..." diff --git a/packages/cli/src/assets/scripts/ao-update.sh b/packages/cli/src/assets/scripts/ao-update.sh index 095085b75..1509d2ada 100755 --- a/packages/cli/src/assets/scripts/ao-update.sh +++ b/packages/cli/src/assets/scripts/ao-update.sh @@ -19,7 +19,7 @@ while [ $# -gt 0 ]; do Usage: ao update [--skip-smoke] [--smoke-only] Fast-forwards the local Agent Orchestrator install repo to main, installs deps, -clean-rebuilds critical packages, refreshes the ao launcher, and runs smoke tests. +clean-rebuilds all workspace packages, refreshes the ao launcher, and runs smoke tests. Options: --skip-smoke Skip smoke tests after rebuild @@ -40,7 +40,38 @@ if [ "$SKIP_SMOKE" = true ] && [ "$SMOKE_ONLY" = true ]; then exit 1 fi -REPO_ROOT="${AO_REPO_ROOT:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}" +is_repo_root() { + local candidate="$1" + [ -f "$candidate/packages/ao/bin/ao.js" ] && [ -d "$candidate/packages/cli" ] +} + +find_repo_root_from() { + local dir="$1" + while [ -n "$dir" ] && [ "$dir" != "/" ]; do + if is_repo_root "$dir"; then + printf '%s\n' "$dir" + return 0 + fi + dir="$(dirname "$dir")" + done + return 1 +} + +resolve_repo_root() { + if [ -n "${AO_REPO_ROOT:-}" ]; then + printf '%s\n' "$AO_REPO_ROOT" + return 0 + fi + + local script_dir + script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + find_repo_root_from "$script_dir" || find_repo_root_from "$PWD" +} + +if ! REPO_ROOT="$(resolve_repo_root)"; then + printf 'Unable to find Agent Orchestrator repo root. Fix: run via ao update or set AO_REPO_ROOT.\n' >&2 + exit 1 +fi require_command() { local name="$1" @@ -186,13 +217,8 @@ if [ "$SMOKE_ONLY" = false ]; then run_cmd git pull --ff-only "$UPDATE_REMOTE" "$TARGET_BRANCH" run_cmd pnpm install - run_cmd pnpm --filter @aoagents/ao-core clean - run_cmd pnpm --filter @aoagents/ao-cli clean - run_cmd pnpm --filter @aoagents/ao-web clean - - run_cmd pnpm --filter @aoagents/ao-core build - run_cmd pnpm --filter @aoagents/ao-cli build - run_cmd pnpm --filter @aoagents/ao-web build + run_cmd pnpm -r --if-present clean + run_cmd pnpm build printf '\nRefreshing ao launcher...\n' ( diff --git a/packages/web/src/__tests__/version-update-api.test.ts b/packages/web/src/__tests__/version-update-api.test.ts index e7bd8da63..e03ac35ce 100644 --- a/packages/web/src/__tests__/version-update-api.test.ts +++ b/packages/web/src/__tests__/version-update-api.test.ts @@ -3,7 +3,9 @@ import { NextRequest } from "next/server"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { EventEmitter } from "node:events"; import type * as AoCoreType from "@aoagents/ao-core"; +import type * as ChildProcessType from "node:child_process"; // Use a real on-disk cache file in a per-test temp dir rather than mocking // node:fs. Mocking ESM-imported fs functions is unreliable when the route @@ -27,14 +29,34 @@ const { mockSessionList } = vi.hoisted(() => ({ mockSessionList: vi.fn(async () => [] as Array<{ id: string; status: string }>), })); +const { mockSpawn } = vi.hoisted(() => ({ + mockSpawn: vi.fn(), +})); + +vi.mock("node:child_process", async () => { + const actual = await vi.importActual("node:child_process"); + return { + ...actual, + default: { ...actual, spawn: (...args: unknown[]) => mockSpawn(...args) }, + spawn: (...args: unknown[]) => mockSpawn(...args), + }; +}); + vi.mock("@/lib/services", () => ({ getServices: vi.fn(async () => ({ sessionManager: { list: mockSessionList }, })), })); -import { GET as versionGET } from "@/app/api/version/route"; -import { POST as updatePOST } from "@/app/api/update/route"; +async function versionGET() { + const { GET } = await import("@/app/api/version/route"); + return GET(); +} + +async function updatePOST(req: NextRequest) { + const { POST } = await import("@/app/api/update/route"); + return POST(req); +} // ── Tests ───────────────────────────────────────────────────────────── @@ -174,9 +196,15 @@ describe("GET /api/version", () => { }); describe("POST /api/update", () => { + let mockChildUnref = vi.fn(); + beforeEach(() => { vi.clearAllMocks(); mockSessionList.mockResolvedValue([]); + mockChildUnref = vi.fn(); + const child = new EventEmitter() as EventEmitter & { unref: ReturnType }; + child.unref = mockChildUnref; + mockSpawn.mockReturnValue(child); }); afterEach(() => { @@ -198,6 +226,7 @@ describe("POST /api/update", () => { expect(body.ok).toBe(false); expect(body.activeSessions).toBe(2); expect(body.message).toMatch(/ao stop/); + expect(mockSpawn).not.toHaveBeenCalled(); }); it.each(["working", "idle", "needs_input", "stuck"])( @@ -215,19 +244,35 @@ describe("POST /api/update", () => { { id: "s2", status: "terminated" }, ]); const res = await updatePOST(makeReq()); - // 202 because the guard passed and spawn ran (or failed silently — either - // way the route returns 202 since spawn errors are caught). - expect([202, 500]).toContain(res.status); + expect(res.status).toBe(202); + expect(mockSpawn).toHaveBeenCalledWith( + "ao", + ["update"], + expect.objectContaining({ + detached: true, + stdio: "ignore", + env: expect.objectContaining({ AO_NON_INTERACTIVE_INSTALL: "1" }), + }), + ); + expect(mockChildUnref).toHaveBeenCalledTimes(1); }); it("returns 202 when no sessions are active", async () => { mockSessionList.mockResolvedValue([]); const res = await updatePOST(makeReq()); - expect([202, 500]).toContain(res.status); - if (res.status === 202) { - const body = (await res.json()) as { ok: boolean }; - expect(body.ok).toBe(true); - } + expect(res.status).toBe(202); + const body = (await res.json()) as { ok: boolean }; + expect(body.ok).toBe(true); + expect(mockSpawn).toHaveBeenCalledWith( + "ao", + ["update"], + expect.objectContaining({ + detached: true, + stdio: "ignore", + env: expect.objectContaining({ AO_NON_INTERACTIVE_INSTALL: "1" }), + }), + ); + expect(mockChildUnref).toHaveBeenCalledTimes(1); }); it("returns 500 when session listing throws", async () => {