From beb0e7b799ca07da9088e91985b70de334eb9e28 Mon Sep 17 00:00:00 2001 From: ShiroKSH Date: Wed, 8 Jul 2026 19:52:23 +0300 Subject: [PATCH] fix: handle Windows install and review edge cases --- apps/amp-plugin/plannotator.test.ts | 4 +- apps/amp-plugin/plannotator.ts | 48 ++++++++------- apps/pi-extension/server.test.ts | 31 ++++++++++ apps/pi-extension/server/serverPlan.ts | 5 +- packages/ui/hooks/useAIChat.seam.test.tsx | 64 ++++++++++++++++++++ packages/ui/hooks/useAIChat.ts | 7 ++- packages/ui/hooks/useUpdateCheck.ts | 3 +- packages/ui/utils/sharing.ts | 1 - scripts/install.ps1 | 72 +++++++++++------------ scripts/install.test.ts | 57 ++++++++++-------- 10 files changed, 203 insertions(+), 89 deletions(-) diff --git a/apps/amp-plugin/plannotator.test.ts b/apps/amp-plugin/plannotator.test.ts index 0d4f59f9d..786121571 100644 --- a/apps/amp-plugin/plannotator.test.ts +++ b/apps/amp-plugin/plannotator.test.ts @@ -262,8 +262,8 @@ describe("Amp Plannotator plugin helpers", () => { }, }), ).toEqual([ - [String.raw`C:\Users\alice\AppData\Local/plannotator/plannotator.exe`], - [String.raw`C:\Users\alice/.local/bin/plannotator.exe`], + [String.raw`C:\Users\alice\AppData\Local\plannotator\plannotator.exe`], + [String.raw`C:\Users\alice\.local\bin\plannotator.exe`], ["plannotator"], ]); }); diff --git a/apps/amp-plugin/plannotator.ts b/apps/amp-plugin/plannotator.ts index 91903f3c8..20bcb818d 100644 --- a/apps/amp-plugin/plannotator.ts +++ b/apps/amp-plugin/plannotator.ts @@ -1,7 +1,8 @@ import type { PluginAPI, PluginCommandContext, ThreadMessage } from "@ampcode/plugin"; import { existsSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; -import { basename, dirname, join, resolve } from "node:path"; +import { dirname, join, posix, resolve, win32 } from "node:path"; +import { fileURLToPath as nodeFileURLToPath } from "node:url"; import { randomUUID } from "node:crypto"; const CATEGORY = "Plannotator"; @@ -529,11 +530,7 @@ function normalizeWorkspaceRoot(value: unknown): string | null { function fileUrlToPath(value: string): string { const url = new URL(value); if (url.protocol !== "file:") throw new Error(`Unsupported URL protocol: ${url.protocol}`); - - const pathname = decodeURIComponent(url.pathname); - return process.platform === "win32" && /^\/[A-Za-z]:/.test(pathname) - ? pathname.slice(1) - : pathname; + return nodeFileURLToPath(url); } export function buildPlannotatorEnv(cwd: string, readyFile: string | null): Record { @@ -549,7 +546,7 @@ function normalizeDirectory(value: string | undefined): string | null { if (!candidate || candidate === "undefined" || candidate === "null") return null; try { - return statSync(candidate).isDirectory() ? candidate : null; + return statSync(candidate).isDirectory() ? resolve(candidate) : null; } catch { return null; } @@ -710,8 +707,9 @@ export function getPlannotatorCommandCandidates( } = {}, ): string[][] { const env = options.env ?? process.env; - const homes = getHomeDirectoryCandidates(env, options.home, options.pluginDir ?? import.meta.dir); const platform = options.platform ?? process.platform; + const homes = getHomeDirectoryCandidates(env, options.home, options.pluginDir ?? import.meta.dir, platform); + const pathJoin = joinForPlatform(platform); const candidates: string[][] = []; const explicitBin = normalizeExecutablePath(env.PLANNOTATOR_BIN); @@ -719,14 +717,14 @@ export function getPlannotatorCommandCandidates( if (platform === "win32") { const localAppData = normalizeExecutablePath(env.LOCALAPPDATA); - if (localAppData) candidates.push([join(localAppData, "plannotator", "plannotator.exe")]); + if (localAppData) candidates.push([pathJoin(localAppData, "plannotator", "plannotator.exe")]); for (const home of homes) { - candidates.push([join(home, ".local", "bin", "plannotator.exe")]); + candidates.push([pathJoin(home, ".local", "bin", "plannotator.exe")]); } } else { for (const home of homes) { - candidates.push([join(home, ".local", "bin", "plannotator")]); + candidates.push([pathJoin(home, ".local", "bin", "plannotator")]); } } @@ -744,32 +742,42 @@ function getHomeDirectoryCandidates( env: Record, explicitHome: string | undefined, pluginDir: string, + platform: string, ): string[] { return dedupeStrings([ normalizeExecutablePath(explicitHome), normalizeExecutablePath(env.HOME), normalizeExecutablePath(env.USERPROFILE), - deriveHomeFromAmpPluginDir(pluginDir), + deriveHomeFromAmpPluginDir(pluginDir, platform), explicitHome === undefined ? normalizeExecutablePath(homedir()) : null, ]); } -function deriveHomeFromAmpPluginDir(pluginDir: string): string | null { - const pluginsDir = resolve(pluginDir); - const ampDir = dirname(pluginsDir); - const configDir = dirname(ampDir); +function deriveHomeFromAmpPluginDir(pluginDir: string, platform: string): string | null { + const pathApi = pathApiForPlatform(platform); + const pluginsDir = pathApi.resolve(pluginDir); + const ampDir = pathApi.dirname(pluginsDir); + const configDir = pathApi.dirname(ampDir); if ( - basename(pluginsDir) === "plugins" && - basename(ampDir) === "amp" && - basename(configDir) === ".config" + pathApi.basename(pluginsDir) === "plugins" && + pathApi.basename(ampDir) === "amp" && + pathApi.basename(configDir) === ".config" ) { - return dirname(configDir); + return pathApi.dirname(configDir); } return null; } +function pathApiForPlatform(platform: string): typeof posix | typeof win32 { + return platform === "win32" ? win32 : posix; +} + +function joinForPlatform(platform: string): typeof posix.join { + return pathApiForPlatform(platform).join; +} + function getAmpCacheDir(): string { const cacheHome = normalizeExecutablePath(process.env.XDG_CACHE_HOME); return cacheHome ? join(cacheHome, "amp") : join(homedir(), ".cache", "amp"); diff --git a/apps/pi-extension/server.test.ts b/apps/pi-extension/server.test.ts index f5afcaa0e..43b2b1927 100644 --- a/apps/pi-extension/server.test.ts +++ b/apps/pi-extension/server.test.ts @@ -1007,6 +1007,37 @@ describe("pi review server", () => { }, 20_000); }); +describe("pi plan archive server", () => { + test("serves an empty archived plan as a found plan", async () => { + const archiveDir = makeTempDir("plannotator-pi-archive-"); + const filename = "2026-01-02-empty-approved.md"; + writeFileSync(join(archiveDir, filename), "", "utf-8"); + process.env.PLANNOTATOR_PORT = String(await reservePort()); + + const server = await startPlanReviewServer({ + plan: "", + origin: "pi", + htmlContent: "archive", + mode: "archive", + customPlanPath: archiveDir, + }); + + try { + const url = new URL(`${server.url}/api/archive/plan`); + url.searchParams.set("filename", filename); + url.searchParams.set("customPath", archiveDir); + const response = await fetch(url); + const payload = await response.json() as { markdown?: string; filepath?: string }; + + expect(response.status).toBe(200); + expect(payload.markdown).toBe(""); + expect(payload.filepath).toBe(filename); + } finally { + server.stop(); + } + }); +}); + describe("pi plan server file browser", () => { test("filters excluded folders from tree and workspace status", async () => { const repo = makeTempDir("plannotator-pi-files-"); diff --git a/apps/pi-extension/server/serverPlan.ts b/apps/pi-extension/server/serverPlan.ts index d1b4c985f..e56dc0a05 100644 --- a/apps/pi-extension/server/serverPlan.ts +++ b/apps/pi-extension/server/serverPlan.ts @@ -184,7 +184,7 @@ export async function startPlanReviewServer(options: { return; } const markdown = readArchivedPlan(filename, customPath); - if (!markdown) { + if (markdown === null) { json(res, { error: "Not found" }, 404); return; } @@ -251,11 +251,12 @@ export async function startPlanReviewServer(options: { }); } else if (url.pathname === "/api/config" && req.method === "POST") { try { - const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; pfmReminder?: boolean }; + const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean; conventionalLabels?: unknown[] | null; pfmReminder?: boolean }; const toSave: Record = {}; if (body.displayName !== undefined) toSave.displayName = body.displayName; if (body.diffOptions !== undefined) toSave.diffOptions = body.diffOptions; if (body.conventionalComments !== undefined) toSave.conventionalComments = body.conventionalComments; + if (body.conventionalLabels !== undefined) toSave.conventionalLabels = body.conventionalLabels; if (body.pfmReminder !== undefined) toSave.pfmReminder = body.pfmReminder; if (Object.keys(toSave).length > 0) saveConfig(toSave as Parameters[0]); json(res, { ok: true }); diff --git a/packages/ui/hooks/useAIChat.seam.test.tsx b/packages/ui/hooks/useAIChat.seam.test.tsx index 6466eaa01..7de9f4580 100644 --- a/packages/ui/hooks/useAIChat.seam.test.tsx +++ b/packages/ui/hooks/useAIChat.seam.test.tsx @@ -52,6 +52,19 @@ function makeSseResponse(textDelta: string): Response { }); } +function makeAbortableSseResponse(signal: AbortSignal): Response { + return new Response(new ReadableStream({ + start(controller) { + signal.addEventListener('abort', () => { + controller.error(new DOMException('aborted', 'AbortError')); + }, { once: true }); + }, + }), { + status: 200, + headers: { 'Content-Type': 'text/event-stream' }, + }); +} + async function mountHook(context: AIContext | null): Promise<{ result: { current: HookResult | null }; unmount: () => Promise; @@ -73,6 +86,49 @@ async function mountHook(context: AIContext | null): Promise<{ }; } +async function expectResetAbortsActiveTurn(resetKind: 'session' | 'thread'): Promise { + const abortBodies: unknown[] = []; + let resolveQueryStarted: () => void = () => {}; + const queryStarted = new Promise((resolve) => { + resolveQueryStarted = resolve; + }); + + const fakeTransport: AITransport = { + session: async () => new Response(JSON.stringify({ sessionId: 'fake-session-reset' }), { status: 200 }), + query: async (_body, signal) => { + resolveQueryStarted(); + return makeAbortableSseResponse(signal); + }, + abort: async (body) => { abortBodies.push(body); }, + permission: () => {}, + }; + + setAITransport(fakeTransport); + + const session = await mountHook(TEST_CONTEXT); + let askPromise: Promise = Promise.resolve(); + + await act(async () => { + askPromise = session.result.current!.ask({ prompt: 'slow question' }); + }); + await queryStarted; + + await act(async () => { + if (resetKind === 'session') { + session.result.current!.resetSession(); + } else { + session.result.current!.resetThread(); + } + }); + + await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); + await askPromise; + + expect(abortBodies).toEqual([{ sessionId: 'fake-session-reset' }]); + + await session.unmount(); +} + describe('AITransport seam', () => { test.skipIf(!hasDom)('fake session + query are called when useAIChat.ask() is invoked', async () => { const sessionBodies: unknown[] = []; @@ -148,4 +204,12 @@ describe('AITransport seam', () => { await session.unmount(); }); + + test.skipIf(!hasDom)('resetSession aborts the active server turn', async () => { + await expectResetAbortsActiveTurn('session'); + }); + + test.skipIf(!hasDom)('resetThread aborts the active server turn', async () => { + await expectResetAbortsActiveTurn('thread'); + }); }); diff --git a/packages/ui/hooks/useAIChat.ts b/packages/ui/hooks/useAIChat.ts index ec85fc28f..3bd7a455e 100644 --- a/packages/ui/hooks/useAIChat.ts +++ b/packages/ui/hooks/useAIChat.ts @@ -224,6 +224,7 @@ export function useAIChat({ }, []); const setSessionId = useCallback((sessionId: string | null) => { + sessionIdRef.current = sessionId; setThread(prev => ({ ...prev, sessionId })); }, []); @@ -486,11 +487,12 @@ export function useAIChat({ if (abortRef.current) { abortRef.current.abort(); abortRef.current = null; + pendingAbortRef.current = postServerAbort(); } setSessionId(null); setIsCreatingSession(false); setIsStreaming(false); - }, [setSessionId]); + }, [postServerAbort, setSessionId]); const resetThread = useCallback(() => { sessionEpochRef.current += 1; @@ -498,12 +500,13 @@ export function useAIChat({ if (abortRef.current) { abortRef.current.abort(); abortRef.current = null; + pendingAbortRef.current = postServerAbort(); } setThread(createThread(threadTitle)); setIsCreatingSession(false); setIsStreaming(false); setError(null); - }, [threadTitle]); + }, [postServerAbort, threadTitle]); useEffect(() => { return () => { diff --git a/packages/ui/hooks/useUpdateCheck.ts b/packages/ui/hooks/useUpdateCheck.ts index 94808997e..ab9e7203c 100644 --- a/packages/ui/hooks/useUpdateCheck.ts +++ b/packages/ui/hooks/useUpdateCheck.ts @@ -114,8 +114,7 @@ export function useUpdateCheck(): UpdateInfo | null { releaseUrl: release.html_url, featureHighlight, }); - } catch (e) { - console.debug('Update check failed:', e); + } catch { } }; diff --git a/packages/ui/utils/sharing.ts b/packages/ui/utils/sharing.ts index 998a29150..921710ea7 100644 --- a/packages/ui/utils/sharing.ts +++ b/packages/ui/utils/sharing.ts @@ -298,7 +298,6 @@ export async function createShortShareUrl( } // Service unavailable — expected for self-hosted setups without a paste backend. // The caller is responsible for falling back to hash-based sharing silently. - console.debug('[sharing] Short URL service unavailable, using hash-based sharing:', e); return null; } } diff --git a/scripts/install.ps1 b/scripts/install.ps1 index f1e37b528..bcfbcbac4 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -32,7 +32,7 @@ if ($Minimal -and $NoMinimal) { } # Binary-only mode. Installs just the plannotator binary and no persistent state -# elsewhere — no sem sidecar, agent-terminal runtime, skills, hooks, or per-agent +# elsewhere - no sem sidecar, agent-terminal runtime, skills, hooks, or per-agent # config. Precedence: -Minimal / -NoMinimal switch > PLANNOTATOR_MINIMAL env var # > default (off). Mirrors install.sh's --minimal / --no-minimal. $minimal = $false @@ -48,13 +48,13 @@ $semVersion = "v0.8.0" $installDir = "$env:LOCALAPPDATA\plannotator" # First plannotator release that carries SLSA build-provenance attestations. -# See scripts/install.sh for the full explanation — this constant is bumped +# See scripts/install.sh for the full explanation - this constant is bumped # once at the first attested release via the release skill. $minAttestedVersion = "v0.17.2" # Detect architecture. Native ARM64 Windows binaries are built from # bun-windows-arm64 (stable since Bun v1.3.10), so ARM64 hosts get a -# native binary — no Windows x86-64 emulation tax. +# native binary - no Windows x86-64 emulation tax. # # PROCESSOR_ARCHITECTURE reports the architecture the current PowerShell # process is running under. PROCESSOR_ARCHITEW6432 is set only in 32-bit @@ -64,7 +64,7 @@ $minAttestedVersion = "v0.17.2" if (-not [Environment]::Is64BitOperatingSystem) { # Write-Error under $ErrorActionPreference = "Stop" (set at the top # of this file) raises a terminating error that exits the process - # with code 1. No explicit `exit 1` needed here — it would be + # with code 1. No explicit `exit 1` needed here - it would be # unreachable. Same applies to every other Write-Error in this file. Write-Error "32-bit Windows is not supported" } @@ -218,13 +218,13 @@ if (Test-Path $configPath) { try { $cfg = Get-Content $configPath -Raw -ErrorAction Stop | ConvertFrom-Json -ErrorAction Stop # Strict check: only a real JSON `true` (parsed as [bool]$true) opts in. - # A stringified "true", a number, etc. do not — matches install.sh, which + # A stringified "true", a number, etc. do not - matches install.sh, which # greps for a literal boolean. if ($cfg.verifyAttestation -is [bool] -and $cfg.verifyAttestation) { $verifyAttestationResolved = $true } } catch { - # Malformed config — ignore, fall through to other layers. + # Malformed config - ignore, fall through to other layers. } } @@ -250,7 +250,7 @@ if ($SkipAttestation) { $verifyAttestationResolved = $false } # v0.9.0 vs v0.10.0 backwards). if ($verifyAttestationResolved) { # Pre-release and build-metadata tags (e.g. v0.18.0-rc1) are not - # supported by [System.Version] — the cast throws on any `-` suffix. + # supported by [System.Version] - the cast throws on any `-` suffix. # install.sh handles these correctly via `sort -V`; Windows has no # built-in semver comparator, so we detect and reject explicitly # with an accurate error rather than surfacing a confusing "could @@ -312,16 +312,16 @@ if ($verifyAttestationResolved) { # MIN_ATTESTED_VERSION pre-flight already rejected older tags. At this # point we know the tag is attested and gh should find a bundle. if (Get-Command gh -ErrorAction SilentlyContinue) { - # Constrain verification to the exact tag + signing workflow — see + # Constrain verification to the exact tag + signing workflow - see # install.sh comment for rationale. $verifyOutput = & gh attestation verify $tmpFile ` --repo $repo ` --source-ref "refs/tags/$latestTag" ` --signer-workflow "backnotprop/plannotator/.github/workflows/release.yml" 2>&1 if ($LASTEXITCODE -eq 0) { - Write-Host "✓ verified build provenance (SLSA)" + Write-Host "Verified build provenance (SLSA)" } else { - # Write to stderr directly — Write-Host goes to PowerShell's + # Write to stderr directly - Write-Host goes to PowerShell's # Information stream, which is silently dropped when callers # redirect stderr for error reporting in CI/CD pipelines. # @@ -365,7 +365,7 @@ function Show-PathAdvice { # Binary-only mode stops here (see the $minimal resolution near the top): the # binary is installed, so add it to PATH and exit before any sidecar download, # agent integration, skill checkout, config write, or cleanup runs. Only the -# binary and its PATH entry are added — none of the sem sidecar, agent-terminal +# binary and its PATH entry are added - none of the sem sidecar, agent-terminal # runtime, or per-agent skills, hooks, or config. if ($minimal) { Show-PathAdvice @@ -477,12 +477,12 @@ function Update-PiExtensionIfPresent { # Aggressive cleanup of stale install locations from prior versions. # Echo each removal and ignore anything that is already gone. -# NOTE: legacy Claude command cleanup happens AFTER the skill install below — +# NOTE: legacy Claude command cleanup happens AFTER the skill install below - # a command file is only removed once its replacement skill is on disk, so a # failed or skipped skill install never leaves users with neither. $claudeCommandsDir = if ($env:CLAUDE_CONFIG_DIR) { "$env:CLAUDE_CONFIG_DIR\commands" } else { "$env:USERPROFILE\.claude\commands" } -# NOTE: Codex stale-skill cleanup happens AFTER the skill install below — the +# NOTE: Codex stale-skill cleanup happens AFTER the skill install below - the # core skills are only removed from the Codex home once their replacement # exists in ~/.agents/skills, so an old pinned tag never strips Codex users # of working skills without a successor. @@ -490,7 +490,7 @@ $staleCodexSkillsDir = Join-Path $codexDir "skills" # Old installers (pre core/extra split) ran a wholesale skills copy against a # new-layout tag and could leave junk `core`/`extra` directory copies in the -# Claude skills scope. Never valid skill names — always safe to remove. +# Claude skills scope. Never valid skill names - always safe to remove. $claudeSkillsScope = if ($env:CLAUDE_CONFIG_DIR) { "$env:CLAUDE_CONFIG_DIR\skills" } else { "$env:USERPROFILE\.claude\skills" } foreach ($junk in @("core", "extra")) { $junkPath = Join-Path $claudeSkillsScope $junk @@ -502,8 +502,8 @@ foreach ($junk in @("core", "extra")) { # Extras (compound / setup-goal / visual-explainer) are no longer managed in # the Claude or shared-agent skill scopes. Remove previously default-installed -# copies ONCE per machine — recorded in the migrations ledger under the -# Plannotator data dir — because copies the user reinstalls via `npx skills +# copies ONCE per machine - recorded in the migrations ledger under the +# Plannotator data dir - because copies the user reinstalls via `npx skills # add` are byte-identical to ours and can only be told apart by remembering # that this cleanup already ran. $claudeSkillsDir = if ($env:CLAUDE_CONFIG_DIR) { "$env:CLAUDE_CONFIG_DIR\skills" } else { "$env:USERPROFILE\.claude\skills" } @@ -543,7 +543,7 @@ if (Test-Path $prefsFile) { } # Extras already on disk (pre-existing or previously npx-installed)? Then the -# extras question is moot — they still count toward the checkbox list, and we +# extras question is moot - they still count toward the checkbox list, and we # never launch the npx flow over them. $extrasPresent = $false foreach ($skill in $extraSkillNames) { @@ -662,10 +662,10 @@ if ($runWizard) { Write-Host "==========================================" Write-Host "" if ($extrasPresent) { - Write-Host "Extra skills already installed — keeping them." + Write-Host "Extra skills already installed - keeping them." $extrasChoice = "yes" } elseif ($Extras -or $NoExtras) { - # Flag already answered this question — don't ask and then ignore. + # Flag already answered this question - don't ask and then ignore. $extrasChoice = if ($Extras) { "yes" } else { "no" } } else { $defaultExtras = if ($savedExtras) { $savedExtras } else { "no" } @@ -674,7 +674,7 @@ if ($runWizard) { $invocableList = $coreSkillNames if ($extrasChoice -eq "yes") { $invocableList = $coreSkillNames + $extraSkillNames } if ($ModelInvocable) { - # Flag already answered this question — don't ask and then ignore. + # Flag already answered this question - don't ask and then ignore. $invocableChoice = $ModelInvocable } else { $wantInvocable = Read-YesNo "Make any skills callable by the model (instead of user-invoked only)?" "no" @@ -693,7 +693,7 @@ if ($ModelInvocable) { $invocableChoice = $ModelInvocable } if (-not $extrasChoice) { $extrasChoice = if ($savedExtras) { $savedExtras } else { "no" } } if (-not $invocableChoice) { $invocableChoice = if ($savedInvocable) { $savedInvocable } else { "none" } } -# Persist only when the wizard ran or a flag set something — silent re-runs +# Persist only when the wizard ran or a flag set something - silent re-runs # must not clobber saved answers with defaults. if ($runWizard -or $Extras -or $NoExtras -or $ModelInvocable) { New-Item -ItemType Directory -Force -Path $configDir | Out-Null @@ -701,14 +701,14 @@ if ($runWizard -or $Extras -or $NoExtras -or $ModelInvocable) { } # Extras install is delegated to the skills CLI (its UI picks the agents). -# Interactive only — silent runs and CI get the printed command instead. +# Interactive only - silent runs and CI get the printed command instead. # Never runs when the extras already exist. if (($extrasChoice -eq "yes") -and (-not $extrasPresent)) { if ($canPrompt -and (Get-Command npx -ErrorAction SilentlyContinue)) { Write-Host "Launching the skills CLI for the extras (pick your agents in its UI)..." npx skills add backnotprop/plannotator/apps/skills/extra if ($LASTEXITCODE -ne 0) { - Write-Host "skills CLI did not complete — install later with: npx skills add backnotprop/plannotator/apps/skills/extra" + Write-Host "skills CLI did not complete - install later with: npx skills add backnotprop/plannotator/apps/skills/extra" } } else { Write-Host "Install the extras with: npx skills add backnotprop/plannotator/apps/skills/extra" @@ -753,7 +753,7 @@ function Copy-SkillIfPresent { try { git clone --depth 1 --filter=blob:none --sparse "https://github.com/$repo.git" --branch $latestTag "$skillsTmp\repo" 2>$null - # git is a native executable — it does not throw under + # git is a native executable - it does not throw under # $ErrorActionPreference=Stop on non-zero exit. Guard with # Test-Path so we only Push-Location if the clone actually # produced a repo directory. @@ -769,10 +769,10 @@ try { # Claude Code and Codex consume different skill bodies. Claude Code # reads apps/skills/claude/* (dynamic-context injection - # `!`plannotator … $ARGUMENTS`` + allowed-tools, so /plannotator-* - # run with no permission prompt — like the old slash commands). + # `!`plannotator ... $ARGUMENTS`` + allowed-tools, so /plannotator-* + # run with no permission prompt - like the old slash commands). # Codex reads apps/skills/core/* (prose the model follows via its - # own shell). The `!`…`` injection is a Claude-Code-only extension, + # own shell). The `!`...`` injection is a Claude-Code-only extension, # so the two are sourced separately rather than sharing one body. # Route each through Copy-SkillIfPresent (which pre-removes the # existing target dir) so re-runs replace rather than nest. @@ -783,7 +783,7 @@ try { } Write-Host "Installed Claude Code skills to $claudeSkillsDir\" } else { - Write-Host "Tag $latestTag predates the per-agent skill layout — skipping Claude Code skill install" + Write-Host "Tag $latestTag predates the per-agent skill layout - skipping Claude Code skill install" } if ((Test-Path "apps\skills\core") -and (Get-ChildItem "apps\skills\core" -ErrorAction SilentlyContinue)) { New-Item -ItemType Directory -Force -Path $agentsSkillsDir | Out-Null @@ -792,7 +792,7 @@ try { } Write-Host "Installed shared agent skills to $agentsSkillsDir\" } else { - Write-Host "Tag $latestTag predates the core/extra skill layout — skipping shared agent skill install" + Write-Host "Tag $latestTag predates the core/extra skill layout - skipping shared agent skill install" } # Kiro: hand-maintained skills (origin baked in) + two extras. @@ -805,7 +805,7 @@ try { # Two extras come from apps/skills/extra (not duplicated into apps/kiro-cli/skills). Copy-SkillIfPresent "apps\skills\extra\plannotator-setup-goal" $kiroSkillsDir Copy-SkillIfPresent "apps\skills\extra\plannotator-visual-explainer" $kiroSkillsDir - # Plannotator custom agent — don't clobber a user's existing one. + # Plannotator custom agent - don't clobber a user's existing one. $kiroAgentsDir = "$env:USERPROFILE\.kiro\agents" if (-not (Test-Path "$kiroAgentsDir\plannotator.json") -and (Test-Path "apps\kiro-cli\agents\plannotator.json")) { New-Item -ItemType Directory -Force -Path $kiroAgentsDir | Out-Null @@ -853,12 +853,12 @@ Remove-Item -Recurse -Force $skillsTmp -ErrorAction SilentlyContinue if ($checkoutFailed) { Write-Host "Error: unable to fetch $repo at $latestTag (network or git error)." - Write-Host "Something went wrong — run the installer again." + Write-Host "Something went wrong - run the installer again." exit 1 } # Claude Code commands are deprecated in favor of skills. Remove a legacy -# command file only once its replacement skill is actually on disk — running +# command file only once its replacement skill is actually on disk - running # AFTER the install above guarantees a failed or skipped skill install never # leaves users with neither the command nor the skill. foreach ($cmd in @("plannotator-review", "plannotator-annotate", "plannotator-last")) { @@ -879,7 +879,7 @@ foreach ($scope in @($claudeSkillsDir, $agentsSkillsDir, "$env:USERPROFILE\.kiro Remove-Item -Recurse -Force $staleArchivePath -ErrorAction SilentlyContinue } } -# The /plannotator-archive OpenCode command was removed too — sweep the stub. +# The /plannotator-archive OpenCode command was removed too - sweep the stub. $staleOpencodeArchive = "$env:USERPROFILE\.config\opencode\commands\plannotator-archive.md" if (Test-Path $staleOpencodeArchive) { Write-Host "Removing stale plannotator-archive command $staleOpencodeArchive" @@ -1056,13 +1056,13 @@ if ($extrasChoice -ne "yes") { } # Warn if plannotator is configured in both settings.json hooks AND the plugin (causes double execution) -# Only warn when the plugin is installed — manual-only users won't have overlap +# Only warn when the plugin is installed - manual-only users won't have overlap $claudeSettings = if ($env:CLAUDE_CONFIG_DIR) { "$env:CLAUDE_CONFIG_DIR\settings.json" } else { "$env:USERPROFILE\.claude\settings.json" } if ((Test-Path $pluginHooks) -and (Test-Path $claudeSettings)) { $settingsContent = Get-Content -Path $claudeSettings -Raw -ErrorAction SilentlyContinue if ($settingsContent -match '"command".*plannotator') { Write-Host "" - Write-Host "⚠️ ⚠️ ⚠️ WARNING: DUPLICATE HOOK DETECTED ⚠️ ⚠️ ⚠️" + Write-Host "!!! WARNING: DUPLICATE HOOK DETECTED !!!" Write-Host "" Write-Host " plannotator was found in your settings.json hooks:" Write-Host " $claudeSettings" @@ -1071,6 +1071,6 @@ if ((Test-Path $pluginHooks) -and (Test-Path $claudeSettings)) { Write-Host " Remove the plannotator hook from settings.json and rely on the" Write-Host " plugin instead (installed automatically via marketplace)." Write-Host "" - Write-Host "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️" + Write-Host "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" } } diff --git a/scripts/install.test.ts b/scripts/install.test.ts index 98eee06ae..d152ca2bb 100644 --- a/scripts/install.test.ts +++ b/scripts/install.test.ts @@ -13,6 +13,10 @@ import { join } from "node:path"; const scriptsDir = import.meta.dir; +function readScript(name: string): string { + return readFileSync(join(scriptsDir, name), "utf-8").replace(/\r\n?/g, "\n"); +} + // The three always-installed core skills (apps/skills/core/*). Single list so // the copy assertions, sidecar checks, and frontmatter checks can't drift. const CORE_SKILLS = [ @@ -22,7 +26,7 @@ const CORE_SKILLS = [ ]; describe("install.sh", () => { - const script = readFileSync(join(scriptsDir, "install.sh"), "utf-8"); + const script = readScript("install.sh"); test("hooks.json heredoc is valid JSON", () => { // Extract the JSON between the HOOKS_EOF heredoc markers @@ -343,7 +347,7 @@ describe("install.sh", () => { }); describe("install.ps1", () => { - const script = readFileSync(join(scriptsDir, "install.ps1"), "utf-8"); + const script = readScript("install.ps1"); test("hooks.json has valid structure", () => { // PS1 uses @"..."@ (interpolated) with $exePathJson for full exe path. @@ -370,6 +374,11 @@ describe("install.ps1", () => { expect(script).toContain("UTF8.GetString"); }); + test("uses only ASCII text so Windows PowerShell can parse UTF-8 without a BOM", () => { + expect(script).toContain('Write-Host "Verified build provenance (SLSA)"'); + expect(script).toMatch(/^[\x00-\x7F]*$/); + }); + test("install.ps1 selects native arm64 binary on ARM64 Windows", () => { // release.yml now builds bun-windows-arm64 (stable since Bun v1.3.10), // so ARM64 hosts get a native binary instead of running the x64 build @@ -512,7 +521,7 @@ describe("install.ps1", () => { }); describe("install.cmd", () => { - const script = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const script = readScript("install.cmd"); test("hooks.json echo block produces valid JSON structure", () => { // The .cmd file uses echo statements to produce JSON. @@ -741,8 +750,8 @@ describe("Core Plannotator skills", () => { }); describe("install shared behavior", () => { - const sh = readFileSync(join(scriptsDir, "install.sh"), "utf-8"); - const ps = readFileSync(join(scriptsDir, "install.ps1"), "utf-8"); + const sh = readScript("install.sh"); + const ps = readScript("install.ps1"); test("install.cmd contains no unix redirect bash-isms", () => { // Tripwire: during PR #850 development, three freshly written `>nul` @@ -750,12 +759,12 @@ describe("install shared behavior", () => { // unidentified external tool. In batch, >/dev/null redirects to a literal // .\dev\null file. If this trips, something between editor and disk is // rewriting cmd syntax. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).not.toContain("/dev/null"); }); test("binary-only (minimal) mode exists in all three installers", () => { - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // Every installer exposes the flag, its --binary-only / -BinaryOnly alias, // the explicit opt-out, and the PLANNOTATOR_MINIMAL env-var fallback — so a // user gets the same binary-only path whatever host they install from. @@ -775,7 +784,7 @@ describe("install shared behavior", () => { }); test("guided install exists in all three installers with safe automation behavior", () => { - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // Shared prefs file (same format across platforms) in the data dir. expect(sh).toContain('PREFS_FILE="$_config_dir/install-prefs"'); expect(ps).toContain('Join-Path $configDir "install-prefs"'); @@ -830,7 +839,7 @@ describe("install shared behavior", () => { test("all installers respect CODEX_HOME for the Codex home directory", () => { // Codex stores config and state under $CODEX_HOME when set, falling back // to ~/.codex (developers.openai.com/codex/config-advanced). #852 - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(sh).toContain('CODEX_DIR="${CODEX_HOME:-$HOME/.codex}"'); expect(ps).toContain('if ($env:CODEX_HOME) { $env:CODEX_HOME } else { "$env:USERPROFILE\\.codex" }'); expect(cmdScript).toContain('if defined CODEX_HOME set "CODEX_DIR=%CODEX_HOME%"'); @@ -842,7 +851,7 @@ describe("install shared behavior", () => { // A --version tag predating apps/skills/core must be diagnosed in every // installer, not just bash — a silent skip leaves Windows users with no // skills and no explanation. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(sh).toContain("predates the core/extra skill layout"); expect(ps).toContain("predates the core/extra skill layout"); expect(cmdScript).toContain("predates the core/extra skill layout"); @@ -892,7 +901,7 @@ describe("install shared behavior", () => { // VERSION with "stray" // Same pair of bugs existed in install.cmd. Both scripts now track // VERSION_EXPLICIT and dash-check the value after --version. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // install.sh expect(sh).toContain("VERSION_EXPLICIT=0"); @@ -931,7 +940,7 @@ describe("install shared behavior", () => { // line order; ps1 took a fixed SkipAttestation-always-wins). No sane // user passes both, so the right behavior is to reject the ambiguous // combination upfront with a clean "mutually exclusive" error. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // install.sh — guards in both --verify-attestation and --skip-attestation arms expect(sh).toContain("mutually exclusive"); @@ -948,7 +957,7 @@ describe("install shared behavior", () => { // curl's output. Every `-o` target in install.cmd must use %RANDOM%. // Covers release.json, the binary itself, the checksum sidecar, and // the gh attestation output capture. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).toContain("plannotator-release-%RANDOM%.json"); expect(cmdScript).toContain("plannotator-%RANDOM%.exe"); expect(cmdScript).toContain("plannotator-checksum-%RANDOM%.txt"); @@ -968,7 +977,7 @@ describe("install shared behavior", () => { // // This test uses indexOf to assert the resolution block appears // textually BEFORE the download line in each installer. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // install.sh: resolution before curl -o const shResolve = sh.indexOf("verify_attestation=0"); @@ -1001,7 +1010,7 @@ describe("install shared behavior", () => { // environment variables ($env:TAG_NUM, $env:MIN_NUM). PowerShell // reads env var values as raw strings and never parses them as code; // the [version] cast throws on invalid input and catch swallows it. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).toContain("$env:TAG_NUM"); expect(cmdScript).toContain("$env:MIN_NUM"); // The vulnerable interpolation form must be gone. @@ -1017,7 +1026,7 @@ describe("install shared behavior", () => { // from index 1) instead, which is equivalent to stripping the // leading `v` because TAG is guaranteed to start with `v` by the // upstream normalization. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).toContain('set "TAG_NUM=!TAG:~1!"'); expect(cmdScript).toContain('set "MIN_NUM=!MIN_ATTESTED_VERSION:~1!"'); // The global-substitution form must be gone from the pre-flight block. @@ -1037,7 +1046,7 @@ describe("install shared behavior", () => { // error that points users at --skip-attestation or a stable tag. // install.sh handles these correctly via `sort -V` and doesn't need // the pre-check. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).toContain("Pre-release tags"); expect(cmdScript).toContain('if not "!TAG_NUM!"=="!TAG_NUM:-=!"'); expect(ps).toContain("Pre-release tags"); @@ -1062,7 +1071,7 @@ describe("install shared behavior", () => { // contain the assignment form doesn't false-match and shadow the // real declaration. All three current assignments are flush-left // at the top of their respective files. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); const shMatch = sh.match(/^MIN_ATTESTED_VERSION="(v\d+\.\d+\.\d+)"/m); const psMatch = ps.match(/^\$minAttestedVersion\s*=\s*"(v\d+\.\d+\.\d+)"/m); const cmdMatch = cmdScript.match(/^set "MIN_ATTESTED_VERSION=(v\d+\.\d+\.\d+)"/m); @@ -1089,7 +1098,7 @@ describe("install shared behavior", () => { // // The constant is bumped once by the release skill at the first // attested release and then left alone as a permanent floor. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // install.sh expect(sh).toContain('MIN_ATTESTED_VERSION="v0.17.2"'); @@ -1106,7 +1115,7 @@ describe("install shared behavior", () => { }); test("all installers install sem sidecar as a non-fatal optional dependency", () => { - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(sh).toContain('SEM_REPO="Ataraxy-Labs/sem"'); expect(sh).toContain('SEM_VERSION="v0.8.0"'); @@ -1141,7 +1150,7 @@ describe("install shared behavior", () => { }); test("all installers install agent terminal runtime as a non-fatal optional dependency", () => { - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(sh).toContain("install_agent_terminal_runtime"); expect(sh).toContain('"$INSTALL_DIR/plannotator" install-runtime agent-terminal'); @@ -1174,7 +1183,7 @@ describe("install shared behavior", () => { // (apps/opencode-plugin/commands, apps/gemini/commands) instead of being // emitted by heredocs/echoes. This retires the old `^^!` cmd-escaping // regression entirely — the fragile echo lines no longer exist. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); // install.cmd no longer echoes plannotator command bodies. expect(cmdScript).not.toContain("echo ^^!`plannotator"); expect(cmdScript).not.toContain("echo ^^!{plannotator"); @@ -1191,7 +1200,7 @@ describe("install shared behavior", () => { // expanded variable, re-exposing cmd metacharacters (& | > <) in // the value before the pipe parses. Must use the safe substring // test pattern used elsewhere in the script. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); expect(cmdScript).toContain('if not "!TAG:~0,1!"=="v"'); expect(cmdScript).not.toContain("echo !TAG! | findstr"); }); @@ -1202,7 +1211,7 @@ describe("install shared behavior", () => { // misattached asset from a different release would pass; without // --signer-workflow an attestation from an unrelated workflow in // the same repo would pass. GitHub's own docs recommend both. - const cmdScript = readFileSync(join(scriptsDir, "install.cmd"), "utf-8"); + const cmdScript = readScript("install.cmd"); for (const [name, script] of [["install.sh", sh], ["install.ps1", ps], ["install.cmd", cmdScript]] as const) { if (!script.includes("--source-ref")) {