fix(cli): persist first-run Codex defaults (#1948)
This commit is contained in:
whoisasx
parent
2fa27412ea
commit
9a6525c085
|
|
@ -17,7 +17,7 @@ import {
|
|||
} from "node:fs";
|
||||
import { basename, join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { parse as parseYaml } from "yaml";
|
||||
import { parse as parseYaml, stringify as stringifyYaml } from "yaml";
|
||||
import { EventEmitter } from "node:events";
|
||||
import {
|
||||
getDefaultRuntime,
|
||||
|
|
@ -2366,7 +2366,7 @@ describe("start command — platform-aware runtime fallback", () => {
|
|||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("start command — autoCreateConfig", () => {
|
||||
it("writes a flat local config and returns the global project identity", async () => {
|
||||
it("generates flat local config and persists an empty global notifiers default", async () => {
|
||||
const { detectEnvironment } = await import("../../src/lib/detect-env.js");
|
||||
vi.mocked(detectEnvironment).mockResolvedValue({
|
||||
isGitRepo: true,
|
||||
|
|
@ -2403,33 +2403,126 @@ describe("start command — autoCreateConfig", () => {
|
|||
expect(existsSync(configPath)).toBe(true);
|
||||
|
||||
const content = readFileSync(configPath, "utf-8");
|
||||
const parsed = parseYaml(content) as {
|
||||
projects?: unknown;
|
||||
runtime?: string;
|
||||
agent?: string;
|
||||
workspace?: string;
|
||||
};
|
||||
expect(parsed.projects).toBeUndefined();
|
||||
expect(parsed.runtime).toBe(getDefaultRuntime());
|
||||
expect(parsed.agent).toBe("claude-code");
|
||||
expect(parsed.workspace).toBe("worktree");
|
||||
const parsed = parseYaml(content) as Record<string, unknown>;
|
||||
expect(parsed["projects"]).toBeUndefined();
|
||||
expect(parsed["defaults"]).toBeUndefined();
|
||||
expect(parsed["agent"]).toBe("claude-code");
|
||||
expect(parsed["workspace"]).toBe("worktree");
|
||||
expect(parsed["runtime"]).toBe(getDefaultRuntime());
|
||||
|
||||
const globalConfigPath = process.env["AO_GLOBAL_CONFIG"]!;
|
||||
const globalContent = readFileSync(globalConfigPath, "utf-8");
|
||||
const globalParsed = parseYaml(globalContent) as {
|
||||
const globalConfigPath = process.env["AO_GLOBAL_CONFIG"];
|
||||
if (!globalConfigPath) throw new Error("AO_GLOBAL_CONFIG should be set in test setup");
|
||||
const globalConfig = parseYaml(readFileSync(globalConfigPath, "utf-8")) as {
|
||||
defaults?: { notifiers?: unknown[] };
|
||||
projects?: Record<string, { path?: string; sessionPrefix?: string }>;
|
||||
};
|
||||
expect(globalParsed.defaults?.notifiers).toEqual(["composio", "desktop"]);
|
||||
expect(globalConfig.defaults?.notifiers).toEqual([]);
|
||||
|
||||
const projectIds = Object.keys(globalParsed.projects ?? {});
|
||||
const projectIds = Object.keys(globalConfig.projects ?? {});
|
||||
expect(projectIds).toHaveLength(1);
|
||||
expect(config.configPath).toBe(configPath);
|
||||
expect(Object.keys(config.projects)).toEqual(projectIds);
|
||||
expect(config.projects[projectIds[0]!]!.path).toBe(realpathSync(tmpDir));
|
||||
});
|
||||
|
||||
it("removes the flat local config when global registration fails", async () => {
|
||||
it("persists Codex first-run selection without erasing existing global notifiers", async () => {
|
||||
createFakeRepo(tmpDir, "https://github.com/ComposioHQ/agent-orchestrator.git");
|
||||
|
||||
const { detectEnvironment } = await import("../../src/lib/detect-env.js");
|
||||
vi.mocked(detectEnvironment).mockResolvedValue({
|
||||
isGitRepo: true,
|
||||
gitRemote: "https://github.com/ComposioHQ/agent-orchestrator.git",
|
||||
ownerRepo: "ComposioHQ/agent-orchestrator",
|
||||
currentBranch: "main",
|
||||
defaultBranch: "main",
|
||||
hasTmux: true,
|
||||
hasGh: true,
|
||||
ghAuthed: true,
|
||||
hasLinearKey: false,
|
||||
hasSlackWebhook: false,
|
||||
});
|
||||
|
||||
const { detectProjectType, generateRulesFromTemplates } =
|
||||
await import("../../src/lib/project-detection.js");
|
||||
vi.mocked(detectProjectType).mockReturnValue({ languages: [], frameworks: [], tools: [] });
|
||||
vi.mocked(generateRulesFromTemplates).mockReturnValue("custom first-run rules");
|
||||
|
||||
const { detectAvailableAgents, detectAgentRuntime } =
|
||||
await import("../../src/lib/detect-agent.js");
|
||||
vi.mocked(detectAvailableAgents).mockResolvedValue([]);
|
||||
vi.mocked(detectAgentRuntime).mockResolvedValue("codex");
|
||||
|
||||
const { findFreePort } = await import("../../src/lib/web-dir.js");
|
||||
vi.mocked(findFreePort).mockResolvedValue(4242);
|
||||
|
||||
mockProcessCwd.mockReturnValue(tmpDir);
|
||||
mockIsHumanCaller.mockReturnValue(false);
|
||||
|
||||
const globalConfigPath = process.env["AO_GLOBAL_CONFIG"];
|
||||
if (!globalConfigPath) throw new Error("AO_GLOBAL_CONFIG should be set in test setup");
|
||||
writeFileSync(
|
||||
globalConfigPath,
|
||||
stringifyYaml(
|
||||
{
|
||||
port: 4242,
|
||||
defaults: {
|
||||
runtime: getDefaultRuntime(),
|
||||
agent: "claude-code",
|
||||
workspace: "worktree",
|
||||
notifiers: ["desktop"],
|
||||
},
|
||||
projects: {},
|
||||
},
|
||||
{ indent: 2 },
|
||||
),
|
||||
);
|
||||
|
||||
const config = await autoCreateConfig(tmpDir);
|
||||
|
||||
const localConfig = parseYaml(
|
||||
readFileSync(join(tmpDir, "agent-orchestrator.yaml"), "utf-8"),
|
||||
) as Record<string, unknown>;
|
||||
expect(localConfig["projects"]).toBeUndefined();
|
||||
expect(localConfig["defaults"]).toBeUndefined();
|
||||
expect(localConfig["agent"]).toBe("codex");
|
||||
expect(localConfig["runtime"]).toBe(getDefaultRuntime());
|
||||
expect(localConfig["workspace"]).toBe("worktree");
|
||||
expect(localConfig["agentRules"]).toBe("custom first-run rules");
|
||||
|
||||
const globalConfig = parseYaml(readFileSync(globalConfigPath, "utf-8")) as {
|
||||
port?: number;
|
||||
defaults?: {
|
||||
runtime?: string;
|
||||
agent?: string;
|
||||
workspace?: string;
|
||||
notifiers?: unknown[];
|
||||
};
|
||||
projects?: Record<string, { path?: string; sessionPrefix?: string }>;
|
||||
};
|
||||
expect(findFreePort).toHaveBeenCalledWith(4242);
|
||||
expect(globalConfig.port).toBe(4242);
|
||||
expect(globalConfig.defaults).toMatchObject({
|
||||
runtime: getDefaultRuntime(),
|
||||
agent: "codex",
|
||||
workspace: "worktree",
|
||||
notifiers: ["desktop"],
|
||||
});
|
||||
|
||||
const entries = Object.entries(globalConfig.projects ?? {});
|
||||
expect(entries).toHaveLength(1);
|
||||
const firstEntry = entries[0];
|
||||
if (!firstEntry) throw new Error("Expected one global project entry");
|
||||
const [projectId, project] = firstEntry;
|
||||
expect(projectId).not.toBe(basename(tmpDir));
|
||||
expect(project.path).toBe(realpathSync(tmpDir));
|
||||
expect(project.sessionPrefix).toBeTruthy();
|
||||
|
||||
expect(Object.keys(config.projects)).toEqual([projectId]);
|
||||
expect(config.defaults.agent).toBe("codex");
|
||||
expect(config.projects[projectId].agent).toBe("codex");
|
||||
});
|
||||
|
||||
it("removes the generated local config if global registration fails", async () => {
|
||||
const { detectEnvironment } = await import("../../src/lib/detect-env.js");
|
||||
vi.mocked(detectEnvironment).mockResolvedValue({
|
||||
isGitRepo: true,
|
||||
|
|
@ -2450,15 +2543,18 @@ describe("start command — autoCreateConfig", () => {
|
|||
const { detectAvailableAgents, detectAgentRuntime } =
|
||||
await import("../../src/lib/detect-agent.js");
|
||||
vi.mocked(detectAvailableAgents).mockResolvedValue([]);
|
||||
vi.mocked(detectAgentRuntime).mockResolvedValue("claude-code");
|
||||
vi.mocked(detectAgentRuntime).mockResolvedValue("codex");
|
||||
|
||||
const { findFreePort } = await import("../../src/lib/web-dir.js");
|
||||
vi.mocked(findFreePort).mockResolvedValue(3000);
|
||||
|
||||
mockProcessCwd.mockReturnValue(tmpDir);
|
||||
mockIsHumanCaller.mockReturnValue(false);
|
||||
|
||||
const callerContext = await import("../../src/lib/caller-context.js");
|
||||
vi.spyOn(callerContext, "isHumanCaller").mockReturnValue(false);
|
||||
|
||||
const globalConfigPath = process.env["AO_GLOBAL_CONFIG"];
|
||||
if (!globalConfigPath) throw new Error("AO_GLOBAL_CONFIG should be set in test setup");
|
||||
writeFileSync(
|
||||
process.env["AO_GLOBAL_CONFIG"]!,
|
||||
globalConfigPath,
|
||||
[
|
||||
"projects:",
|
||||
` ${basename(tmpDir)}:`,
|
||||
|
|
@ -2467,10 +2563,56 @@ describe("start command — autoCreateConfig", () => {
|
|||
].join("\n"),
|
||||
);
|
||||
|
||||
await expect(autoCreateConfig(tmpDir)).rejects.toThrow("already registered");
|
||||
await expect(autoCreateConfig(tmpDir)).rejects.toThrow(
|
||||
"Could not complete first-run config setup",
|
||||
);
|
||||
|
||||
expect(existsSync(join(tmpDir, "agent-orchestrator.yaml"))).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the generated local config when defaults persistence fails after registration", async () => {
|
||||
createFakeRepo(tmpDir, "https://github.com/ComposioHQ/agent-orchestrator.git");
|
||||
|
||||
const { detectEnvironment } = await import("../../src/lib/detect-env.js");
|
||||
vi.mocked(detectEnvironment).mockResolvedValue({
|
||||
isGitRepo: true,
|
||||
gitRemote: "https://github.com/ComposioHQ/agent-orchestrator.git",
|
||||
ownerRepo: "ComposioHQ/agent-orchestrator",
|
||||
currentBranch: "main",
|
||||
defaultBranch: "main",
|
||||
hasTmux: true,
|
||||
hasGh: true,
|
||||
ghAuthed: true,
|
||||
hasLinearKey: false,
|
||||
hasSlackWebhook: false,
|
||||
});
|
||||
|
||||
const { detectProjectType } = await import("../../src/lib/project-detection.js");
|
||||
vi.mocked(detectProjectType).mockReturnValue({ languages: [], frameworks: [], tools: [] });
|
||||
|
||||
const { detectAvailableAgents, detectAgentRuntime } =
|
||||
await import("../../src/lib/detect-agent.js");
|
||||
vi.mocked(detectAvailableAgents).mockResolvedValue([]);
|
||||
vi.mocked(detectAgentRuntime).mockResolvedValue("codex");
|
||||
|
||||
const { findFreePort } = await import("../../src/lib/web-dir.js");
|
||||
vi.mocked(findFreePort).mockResolvedValue(3000);
|
||||
|
||||
mockProcessCwd.mockReturnValue(tmpDir);
|
||||
mockIsHumanCaller.mockReturnValue(false);
|
||||
|
||||
const core = await import("@aoagents/ao-core");
|
||||
const saveGlobalConfigSpy = vi.spyOn(core, "saveGlobalConfig").mockImplementation(() => {
|
||||
throw new Error("persist failed");
|
||||
});
|
||||
|
||||
await expect(autoCreateConfig(tmpDir)).rejects.toThrow(
|
||||
"Could not complete first-run config setup",
|
||||
);
|
||||
|
||||
expect(saveGlobalConfigSpy).toHaveBeenCalled();
|
||||
expect(existsSync(join(tmpDir, "agent-orchestrator.yaml"))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -35,6 +35,9 @@ import {
|
|||
recordActivityEvent,
|
||||
registerProjectInGlobalConfig,
|
||||
getGlobalConfigPath,
|
||||
loadGlobalConfig,
|
||||
saveGlobalConfig,
|
||||
createDefaultGlobalConfig,
|
||||
type OrchestratorConfig,
|
||||
type LocalProjectConfig,
|
||||
type ProjectConfig,
|
||||
|
|
@ -141,6 +144,36 @@ function writeProjectBehaviorConfig(projectPath: string, config: LocalProjectCon
|
|||
writeLocalProjectConfig(projectPath, config);
|
||||
}
|
||||
|
||||
function persistFirstRunGlobalDefaults(params: {
|
||||
port: number;
|
||||
runtime: string;
|
||||
agent: string;
|
||||
workspace: string;
|
||||
notifiers: string[];
|
||||
preserveExistingNotifiers: boolean;
|
||||
}): void {
|
||||
const globalConfigPath = getGlobalConfigPath();
|
||||
const existing = loadGlobalConfig(globalConfigPath);
|
||||
const baseConfig = existing ?? createDefaultGlobalConfig();
|
||||
saveGlobalConfig(
|
||||
{
|
||||
...baseConfig,
|
||||
port: params.port,
|
||||
defaults: {
|
||||
...baseConfig.defaults,
|
||||
runtime: params.runtime,
|
||||
agent: params.agent,
|
||||
workspace: params.workspace,
|
||||
notifiers:
|
||||
params.preserveExistingNotifiers && existing
|
||||
? [...baseConfig.defaults.notifiers]
|
||||
: [...params.notifiers],
|
||||
},
|
||||
},
|
||||
globalConfigPath,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a flat local config (agent-orchestrator.yaml without `projects:`)
|
||||
* into the global config so loadConfig can resolve it.
|
||||
|
|
@ -566,10 +599,24 @@ export async function autoCreateConfig(workingDir: string): Promise<Orchestrator
|
|||
const agent = await detectAgentRuntime(detectedAgents);
|
||||
console.log(chalk.green(` ✓ Agent runtime: ${agent}`));
|
||||
|
||||
const existingGlobalConfig = loadGlobalConfig(getGlobalConfigPath());
|
||||
const requestedDashboardPort = existingGlobalConfig?.port ?? DEFAULT_PORT;
|
||||
const port = await findFreePort(requestedDashboardPort);
|
||||
const dashboardPort = port ?? requestedDashboardPort;
|
||||
if (port !== null && port !== requestedDashboardPort) {
|
||||
console.log(
|
||||
chalk.yellow(` ⚠ Port ${requestedDashboardPort} is busy — using ${port} instead.`),
|
||||
);
|
||||
}
|
||||
|
||||
const runtime = getDefaultRuntime();
|
||||
const workspace = "worktree";
|
||||
const notifiers: string[] = [];
|
||||
const preserveExistingGlobalNotifiers = existingGlobalConfig !== null;
|
||||
const localConfig: LocalProjectConfig = {
|
||||
runtime: getDefaultRuntime(),
|
||||
runtime,
|
||||
agent,
|
||||
workspace: "worktree",
|
||||
workspace,
|
||||
...(agentRules ? { agentRules } : {}),
|
||||
};
|
||||
|
||||
|
|
@ -579,22 +626,39 @@ export async function autoCreateConfig(workingDir: string): Promise<Orchestrator
|
|||
console.log(chalk.dim(" Use 'ao start' to start with the existing config.\n"));
|
||||
return loadConfig(outputPath);
|
||||
}
|
||||
writeLocalProjectConfig(workingDir, localConfig, outputPath);
|
||||
|
||||
let registeredProjectId: string | null = null;
|
||||
try {
|
||||
const registeredProjectId = registerProjectInGlobalConfig(projectId, projectId, path, {
|
||||
writeLocalProjectConfig(workingDir, localConfig, outputPath);
|
||||
console.log(chalk.green(`✓ Config created: ${outputPath}\n`));
|
||||
|
||||
const sessionPrefix = generateSessionPrefix(projectId);
|
||||
registeredProjectId = registerProjectInGlobalConfig(projectId, projectId, path, {
|
||||
...(repo ? { repo } : {}),
|
||||
defaultBranch,
|
||||
sessionPrefix: generateSessionPrefix(projectId),
|
||||
sessionPrefix,
|
||||
});
|
||||
persistFirstRunGlobalDefaults({
|
||||
port: dashboardPort,
|
||||
runtime,
|
||||
agent,
|
||||
workspace,
|
||||
notifiers,
|
||||
preserveExistingNotifiers: preserveExistingGlobalNotifiers,
|
||||
});
|
||||
|
||||
console.log(chalk.green(`✓ Registered "${registeredProjectId}" in global config\n`));
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
if (!registeredProjectId) {
|
||||
rmSync(outputPath, { force: true });
|
||||
throw err;
|
||||
}
|
||||
|
||||
console.log(chalk.green(`✓ Config created: ${outputPath}\n`));
|
||||
console.log(chalk.yellow("⚠ Could not complete first-run config setup."));
|
||||
console.log(chalk.dim(` ${message}\n`));
|
||||
throw new Error(`Could not complete first-run config setup: ${message}`, {
|
||||
cause: err,
|
||||
});
|
||||
}
|
||||
|
||||
if (!repo) {
|
||||
console.log(
|
||||
|
|
|
|||
Loading…
Reference in New Issue