Skip to content

Commit b2919bc

Browse files
committed
fix: address BugBot findings — ownership repair index alignment and dead code
1. cli/fix.ts: repairOwnership now returns RepairOutcome[] aligned by index with the input issues array, fixing the bug where mixed success/failure results were misaligned via separate fixed[]/failed[] arrays. 2. cli/upgrade.ts + human.ts: Remove unreachable 'channel-only' action variant from UpgradeResult.action union and formatter — no code path produces it. Can be re-added when the feature is implemented.
1 parent 6a5b453 commit b2919bc

3 files changed

Lines changed: 32 additions & 44 deletions

File tree

src/commands/cli/fix.ts

Lines changed: 30 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -269,33 +269,38 @@ function resolveUid(username: string): number | null {
269269
*
270270
* @returns Object with lists of human-readable success and failure messages
271271
*/
272+
/** Per-issue repair outcome, aligned by index with the input issues array */
273+
type RepairOutcome = {
274+
/** Whether the repair succeeded */
275+
success: boolean;
276+
/** Human-readable repair message */
277+
message: string;
278+
};
279+
272280
async function repairOwnership(
273281
issues: OwnershipIssue[],
274282
username: string,
275283
targetUid: number
276-
): Promise<{ fixed: string[]; failed: string[] }> {
277-
const fixed: string[] = [];
278-
const failed: string[] = [];
279-
284+
): Promise<RepairOutcome[]> {
280285
const results = await Promise.allSettled(
281286
issues.map((issue) => chown(issue.path, targetUid, -1))
282287
);
283288

284-
for (let i = 0; i < results.length; i++) {
285-
const result = results[i] as PromiseSettledResult<void>;
289+
return results.map((result, i) => {
286290
const issue = issues[i] as OwnershipIssue;
287291
if (result.status === "fulfilled") {
288-
fixed.push(`${issue.kind} ${issue.path}: transferred to ${username}`);
289-
} else {
290-
const reason =
291-
result.reason instanceof Error
292-
? result.reason.message
293-
: "unknown error";
294-
failed.push(`${issue.kind} ${issue.path}: ${reason}`);
292+
return {
293+
success: true,
294+
message: `${issue.kind} ${issue.path}: transferred to ${username}`,
295+
};
295296
}
296-
}
297-
298-
return { fixed, failed };
297+
const reason =
298+
result.reason instanceof Error ? result.reason.message : "unknown error";
299+
return {
300+
success: false,
301+
message: `${issue.kind} ${issue.path}: ${reason}`,
302+
};
303+
});
299304
}
300305

301306
/** Result of diagnosing a category of issues (used internally) */
@@ -413,34 +418,23 @@ async function handleOwnershipIssues(
413418
// Running as root — perform chown. resolvedTargetUid is guaranteed non-null
414419
// and non-zero here (we bailed out above if it couldn't be resolved).
415420
const resolvedUid = resolvedTargetUid as number;
416-
const { fixed, failed } = await repairOwnership(
417-
rawIssues,
418-
username,
419-
resolvedUid
420-
);
421+
const outcomes = await repairOwnership(rawIssues, username, resolvedUid);
421422

422-
// Mark each issue with its repair outcome
423+
// Mark each issue with its repair outcome (aligned by index)
424+
let anyFailed = false;
423425
for (let i = 0; i < fixIssues.length; i++) {
424426
const issue = fixIssues[i] as FixIssue;
425-
if (i < fixed.length) {
426-
issue.repaired = true;
427-
issue.repairMessage = fixed[i];
427+
const outcome = outcomes[i] as RepairOutcome;
428+
issue.repaired = outcome.success;
429+
issue.repairMessage = outcome.message;
430+
if (!outcome.success) {
431+
anyFailed = true;
428432
}
429433
}
430434

431-
// Append failed repairs as issues with repaired=false
432-
for (const f of failed) {
433-
fixIssues.push({
434-
category: "ownership",
435-
description: f,
436-
repaired: false,
437-
repairMessage: f,
438-
});
439-
}
440-
441435
return {
442436
issues: fixIssues,
443-
repairFailed: failed.length > 0,
437+
repairFailed: anyFailed,
444438
};
445439
}
446440

src/commands/cli/upgrade.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const CHANNEL_VERSIONS = new Set(["nightly", "stable"]);
5858
*/
5959
export type UpgradeResult = {
6060
/** What action was taken */
61-
action: "upgraded" | "downgraded" | "up-to-date" | "checked" | "channel-only";
61+
action: "upgraded" | "downgraded" | "up-to-date" | "checked";
6262
/** Current CLI version before upgrade */
6363
currentVersion: string;
6464
/** Target version (the version we upgraded/downgraded to, or the latest available) */
@@ -120,7 +120,7 @@ type ResolveTargetOptions = {
120120
*
121121
* - `target`: the version string to upgrade/downgrade to (proceed with upgrade)
122122
* - `UpgradeResult`: structured result when no upgrade should proceed
123-
* (check-only mode, already up to date, or channel-only switch)
123+
* (check-only mode, or already up to date)
124124
*/
125125
type ResolveResult =
126126
| { kind: "target"; target: string }

src/lib/formatters/human.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,7 +2018,6 @@ const ACTION_DESCRIPTIONS: Record<UpgradeResult["action"], string> = {
20182018
downgraded: "Downgraded",
20192019
"up-to-date": "Already up to date",
20202020
checked: "Update check complete",
2021-
"channel-only": "Channel updated",
20222021
};
20232022

20242023
/**
@@ -2066,11 +2065,6 @@ export function formatUpgradeResult(data: UpgradeResult): string {
20662065
}
20672066
break;
20682067
}
2069-
case "channel-only":
2070-
lines.push(
2071-
`${colorTag("green", "✓")} Channel set to ${escapeMarkdownInline(data.channel)}`
2072-
);
2073-
break;
20742068
default: {
20752069
// Exhaustive check — all action types should be handled above
20762070
const _: never = data.action;

0 commit comments

Comments
 (0)