diff --git a/src/lib/office.ts b/src/lib/office.ts index 484d5bb..0657c46 100644 --- a/src/lib/office.ts +++ b/src/lib/office.ts @@ -26,7 +26,7 @@ import { legacyOfficeDownloadsDir, officeDownloadsDir } from "@/lib/paths.js"; // installer that prompts for sudo/UAC, which an Ink render would fight over. export const OFFICE_USAGE = - "Usage: codevhub skill office [--platform ubuntu|macos|windows] [--arch arm64|x86_64] [--dir ] [--download-only] [--skip-verify] [--force-skills]"; + "Usage: codevhub skill office [--platform ubuntu|macos|windows] [--arch arm64|x86_64] [--dir ] [--target-user ] [--download-only] [--skip-verify] [--force-skills]"; export type OfficePlatform = "ubuntu" | "macos" | "windows"; export type OfficeArch = "arm64" | "x86_64"; @@ -110,6 +110,13 @@ export interface OfficeArgs { downloadOnly: boolean; skipVerify: boolean; forceSkills: boolean; + // Windows only: the account that will USE the skills, when the installer is + // being run from an admin account instead of theirs. Forwarded to both the + // setup and uninstall scripts as -TargetUser. Without it there was no way to + // drive that case through codevhub at all — the whole point of the flag is + // that a human is running the install manually, which is exactly when they + // reach for codevhub. + targetUser?: string; // Deliberately unadvertised (absent from OFFICE_USAGE and `codevhub help`): // fetches and runs the published uninstall script instead of the installer. uninstall: boolean; @@ -172,6 +179,15 @@ export function parseOfficeArgs(argv: string[]): OfficeArgs { parsed.dir = value; break; } + case "--target-user": { + const value = takeValue(); + if (!value) { + parsed.error = "--target-user requires an account name"; + return parsed; + } + parsed.targetUser = value; + break; + } case "--download-only": parsed.downloadOnly = true; break; @@ -344,6 +360,7 @@ export function installerArgs( // -SkipVerify / -ForceSkills switches. if (platform === "windows") { return [ + ...(parsed.targetUser ? ["-TargetUser", parsed.targetUser] : []), ...(parsed.skipVerify ? ["-SkipVerify"] : []), ...(parsed.forceSkills ? ["-ForceSkills"] : []), ]; @@ -360,6 +377,7 @@ export function uninstallerArgs( ): string[] { if (platform === "windows") { return [ + ...(parsed.targetUser ? ["-TargetUser", parsed.targetUser] : []), ...(parsed.yes ? ["-Yes"] : []), ...(parsed.skillsOnly ? ["-SkillsOnly"] : []), ...(parsed.purgeDownloads ? ["-PurgeDownloads"] : []), @@ -416,6 +434,16 @@ export async function runSkillOffice( return 1; } + // -TargetUser exists only in the PowerShell scripts. On Ubuntu and macOS the + // invoking user is recoverable from SUDO_USER, so there is nothing to name — + // forwarding it would just make the bash script choke on an unknown flag. + if (parsed.targetUser !== undefined && platform !== "windows") { + console.error( + `--target-user only applies to Windows (${platform} resolves the real user from SUDO_USER).`, + ); + return 1; + } + // Never execute a script built for another OS. The override stays useful // for fetching a bundle to carry to a different machine. let downloadOnly = parsed.downloadOnly; @@ -546,8 +574,15 @@ export async function runSkillOffice( console.error(" 2. Copy-paste these two lines into that window:"); console.error(` cd "${dir}"`); console.error(` ${commandLine}`); + // Name the line the target script actually ends with. Uninstall mode used + // to quote the installer's "Verification passed", which the uninstaller + // never prints - so the user sat waiting for a message that was not + // coming. The installer's own closing line is unconditional; its + // "Verification passed" is not, being absent under --skip-verify. console.error( - ' 3. Wait for the green "Verification passed" closing message', + parsed.uninstall + ? ' 3. Wait for the green "codev-office uninstall finished" line' + : ' 3. Wait for the green "codev-office setup complete" closing line', ); logInfo("office windows manual handoff", { action: parsed.uninstall ? "office.uninstall" : "office.install", diff --git a/tests/lib/office.test.ts b/tests/lib/office.test.ts index 4fde395..671ed94 100644 --- a/tests/lib/office.test.ts +++ b/tests/lib/office.test.ts @@ -3,11 +3,14 @@ import { detectArch, detectPlatform, formatSize, + installerArgs, officeBundleName, + officeManualWindowsCommand, officeScriptName, officeTarget, officeUninstallScriptName, parseOfficeArgs, + uninstallerArgs, } from "@/lib/office.js"; describe("detectPlatform", () => { @@ -151,6 +154,101 @@ describe("parseOfficeArgs", () => { test("rejects unknown options", () => { expect(parseOfficeArgs(["--wat"]).error).toMatch(/unknown option: --wat/); }); + + test("--target-user, both spellings", () => { + expect(parseOfficeArgs(["--target-user", "minhnh49"]).targetUser).toBe( + "minhnh49", + ); + expect(parseOfficeArgs(["--target-user=VTS\\minhnh49"]).targetUser).toBe( + "VTS\\minhnh49", + ); + }); + + test("rejects --target-user without a value", () => { + expect(parseOfficeArgs(["--target-user"]).error).toMatch( + /--target-user requires an account name/, + ); + }); + + // It applies to BOTH modes: install puts the skills in the right profile, + // uninstall clears them out of it again. + test("--target-user is legal in install and uninstall mode alike", () => { + expect(parseOfficeArgs(["--target-user", "jdoe"]).error).toBeUndefined(); + expect( + parseOfficeArgs(["--uninstall", "--target-user", "jdoe"]).error, + ).toBeUndefined(); + }); +}); + +// The scripts' own flag names — a drift here silently stops forwarding an +// option the user explicitly asked for. +describe("script argument forwarding", () => { + test("installer: -TargetUser is forwarded as a separate value token", () => { + expect( + installerArgs(parseOfficeArgs(["--target-user", "jdoe"]), "windows"), + ).toEqual(["-TargetUser", "jdoe"]); + }); + + test("installer: combines with the other switches", () => { + expect( + installerArgs( + parseOfficeArgs(["--target-user", "jdoe", "--skip-verify"]), + "windows", + ), + ).toEqual(["-TargetUser", "jdoe", "-SkipVerify"]); + }); + + test("uninstaller: -TargetUser is forwarded alongside its own switches", () => { + expect( + uninstallerArgs( + parseOfficeArgs([ + "--uninstall", + "--target-user", + "jdoe", + "--skills-only", + "--yes", + ]), + "windows", + ), + ).toEqual(["-TargetUser", "jdoe", "-Yes", "-SkillsOnly"]); + }); + + test("nothing is forwarded when the flag is absent", () => { + expect(installerArgs(parseOfficeArgs([]), "windows")).toEqual([]); + expect( + uninstallerArgs(parseOfficeArgs(["--uninstall"]), "windows"), + ).toEqual([]); + }); + + // A domain account (VTS\minhnh49) has no space, but a renamed local account + // can — the printed command has to stay copy-pasteable either way. + test("the printed Windows command quotes a value containing a space", () => { + expect( + officeManualWindowsCommand( + "codev-office-windows-setup.ps1", + installerArgs( + parseOfficeArgs(["--target-user", "First Last"]), + "windows", + ), + ), + ).toBe( + 'powershell -ExecutionPolicy Bypass -File .\\codev-office-windows-setup.ps1 -TargetUser "First Last"', + ); + }); + + test("a domain-qualified account survives unquoted", () => { + expect( + officeManualWindowsCommand( + "codev-office-windows-setup.ps1", + installerArgs( + parseOfficeArgs(["--target-user", "VTS\\minhnh49"]), + "windows", + ), + ), + ).toBe( + "powershell -ExecutionPolicy Bypass -File .\\codev-office-windows-setup.ps1 -TargetUser VTS\\minhnh49", + ); + }); }); // The names are a contract with the codev-scripts repo (codev-office/*) and