Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/lib/office.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path>] [--download-only] [--skip-verify] [--force-skills]";
"Usage: codevhub skill office [--platform ubuntu|macos|windows] [--arch arm64|x86_64] [--dir <path>] [--target-user <name>] [--download-only] [--skip-verify] [--force-skills]";

export type OfficePlatform = "ubuntu" | "macos" | "windows";
export type OfficeArch = "arm64" | "x86_64";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"] : []),
];
Expand All @@ -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"] : []),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand Down
98 changes: 98 additions & 0 deletions tests/lib/office.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ import {
detectArch,
detectPlatform,
formatSize,
installerArgs,
officeBundleName,
officeManualWindowsCommand,
officeScriptName,
officeTarget,
officeUninstallScriptName,
parseOfficeArgs,
uninstallerArgs,
} from "@/lib/office.js";

describe("detectPlatform", () => {
Expand Down Expand Up @@ -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
Expand Down
Loading