From 3ed22893275c0bb83109b35f93052afa2a921938 Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Tue, 11 Aug 2026 08:28:01 +0700 Subject: [PATCH] feat(office): forward --target-user, and name the line the script really ends with Two gaps found while documenting how to repair a Windows machine whose skills were installed into an admin account (codev-landing-page#110). --target-user forwards to both scripts as -TargetUser. It was the one flag codevhub could not express, and the only flag whose entire reason for existing is that a human is driving the install by hand - which is exactly when they reach for codevhub. Without it the documented recovery had to break out of the tool halfway through and tell people to type the second command themselves. It applies to install and uninstall alike: install puts the skills in the right profile, uninstall clears them out of one. Windows only, rejected elsewhere with the same shape as the existing --arch guard, because the bash scripts recover the invoking user from SUDO_USER and would choke on an unknown flag. The Windows manual handoff also told people to "Wait for the green Verification passed closing message" in BOTH modes. The uninstaller never prints that - it ends with "codev-office uninstall finished" - so anyone running --uninstall sat waiting for a message that was not coming. Each mode now names the line its own script actually ends with. For install that is "codev-office setup complete", which is unconditional, rather than "Verification passed", which is absent under --skip-verify. Tests cover both spellings of the flag, the missing-value rejection, legality in both modes, forwarding through installerArgs/uninstallerArgs, and the printed command for a domain-qualified account and for one containing a space. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/office.ts | 39 +++++++++++++++- tests/lib/office.test.ts | 98 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 2 deletions(-) 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