Skip to content
Open
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
17 changes: 11 additions & 6 deletions web/src/components/config-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { cn } from "@/lib/cn";
import { CadenceSettings } from "@/components/followups/cadence-settings";
import { persistCliId, readSavedCliId } from "@/lib/saved-cli";
import { pickDefaultInstalled } from "@/lib/cli-pick.mjs";

type Cli = {
id: string;
Expand Down Expand Up @@ -68,14 +69,18 @@ export function ConfigForm() {
.then((d) => {
const list: Cli[] = d.clis ?? [];
setClis(list);
// Highlight + persist the only installed CLI when Config was never saved.
// Highlight-only used to look configured while jobs still read empty localStorage.
// Highlight + persist the default installed CLI when Config was never
// saved. Highlight-only looks configured while jobs still read empty
// localStorage -- so whatever is rendered as selected must be written.
// This applies at ANY installed count: restricting it to a SOLE install
// left every multi-CLI machine (claude + codex, say) in exactly the
// broken state the highlight was meant to avoid.
setCliId((prev) => {
if (prev) return prev;
const only = list.filter((c) => c.installed);
if (only.length !== 1) return list.find((c) => c.installed)?.id || "";
if (!readSavedCliId()) persistCliId(only[0].id);
return only[0].id;
const auto = pickDefaultInstalled(list);
if (!auto) return "";
if (!readSavedCliId()) persistCliId(auto);
return auto;
});
})
.catch(() => setClis([]));
Expand Down
22 changes: 22 additions & 0 deletions web/src/lib/cli-pick.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Plain .mjs so tests/lib/saved-cli-pick.test.mjs can import the REAL functions
// under `node --test` (no TypeScript loader). saved-cli.ts re-exports these, so
// there is one implementation rather than a mirrored copy in the suite.

/** The single installed CLI, or null when there is not exactly one. */
export function pickSoleInstalled(clis) {
const installed = (clis || []).filter((c) => c && c.installed);
return installed.length === 1 ? installed[0].id : null;
}

/**
* The CLI the Config page should start on: the first installed one, whatever
* the count.
*
* Config renders this as the active choice, so it must also be the value that
* gets persisted. Persisting only the SOLE installed CLI left anyone with two
* or more looking at a selected CLI over empty localStorage -- and every AI
* surface reads that key, so they all silently did nothing.
*/
export function pickDefaultInstalled(clis) {
return (clis || []).find((c) => c && c.installed)?.id || null;
}
9 changes: 3 additions & 6 deletions web/src/lib/saved-cli.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { pickSoleInstalled } from "./cli-pick.mjs";

export const CONFIG_KEY = "career-ops:config";

export function readSavedCliId(): string | null {
Expand All @@ -23,12 +25,7 @@ export function persistCliId(cliId: string) {
}
}

export function pickSoleInstalled(
clis: { id: string; installed?: boolean }[] | undefined,
): string | null {
const installed = (clis || []).filter((c) => c.installed);
return installed.length === 1 ? installed[0].id : null;
}
export { pickDefaultInstalled, pickSoleInstalled } from "./cli-pick.mjs";

/** Saved Config cliId, or the only installed CLI (and persist that pick). */
export async function resolveCliId(): Promise<string | null> {
Expand Down
56 changes: 51 additions & 5 deletions web/tests/lib/saved-cli-pick.test.mjs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import assert from "node:assert/strict";
import { test } from "node:test";

// Mirror of pickSoleInstalled in src/lib/saved-cli.ts (TS; this suite is .mjs).
function pickSoleInstalled(clis) {
const installed = (clis || []).filter((c) => c.installed);
return installed.length === 1 ? installed[0].id : null;
}
// Imports the REAL implementation from src/lib/cli-pick.mjs. This suite used to
// hold a hand-copied mirror of pickSoleInstalled, which cannot fail when the
// real function changes -- exactly the drift that let the multi-CLI gap below
// go unnoticed.
import { pickDefaultInstalled, pickSoleInstalled } from "../../src/lib/cli-pick.mjs";

test("sole installed CLI is the default", () => {
assert.equal(
Expand All @@ -27,3 +27,49 @@ test("zero or two installed CLIs stay unset", () => {
null,
);
});

test("pickSoleInstalled tolerates undefined and sparse entries", () => {
assert.equal(pickSoleInstalled(undefined), null);
assert.equal(pickSoleInstalled([null, { id: "codex", installed: true }]), "codex");
});

// --- pickDefaultInstalled: what Config renders, and therefore must persist ---

test("first installed CLI is the default at ANY installed count", () => {
assert.equal(
pickDefaultInstalled([
{ id: "claude", installed: true },
{ id: "codex", installed: true },
{ id: "gemini", installed: true },
]),
"claude",
);
});

test("the default skips CLIs that are not installed", () => {
assert.equal(
pickDefaultInstalled([
{ id: "claude", installed: false },
{ id: "codex", installed: true },
]),
"codex",
);
});

test("no installed CLI yields null, never an empty-string id", () => {
assert.equal(pickDefaultInstalled([{ id: "claude", installed: false }]), null);
assert.equal(pickDefaultInstalled([]), null);
assert.equal(pickDefaultInstalled(undefined), null);
});

// The regression: a multi-CLI machine used to fall through the SOLE-install
// guard, so Config highlighted a CLI it never wrote. Every AI surface reads
// that key, so all of them silently did nothing.
test("multi-CLI machines get a persistable default, unlike pickSoleInstalled", () => {
const clis = [
{ id: "claude", installed: true },
{ id: "codex", installed: true },
];
assert.equal(pickSoleInstalled(clis), null, "sole-pick correctly declines");
assert.equal(pickDefaultInstalled(clis), "claude", "but a default must still exist to persist");
});
Loading