Skip to content

Fix interactive ao start crash when updating flat local config #2014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yyovil wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix interactive ao start crash when updating flat local config #2014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 94 additions & 0 deletions packages/cli/__tests__/commands/start.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3148,4 +3148,98 @@ describe("start command — global registry mutations", () => {
else process.env["AO_GLOBAL_CONFIG"] = origGlobalEnv;
}
});

it("writes interactive agent overrides to a config that started as flat local format", async () => {
const repoDir = join(tmpDir, "current");
createFakeRepo(repoDir, "https://github.com/org/current.git");
const flatConfigPath = join(repoDir, "agent-orchestrator.yaml");
writeFileSync(
flatConfigPath,
[
"agent: claude-code",
"runtime: process",
"workspace: worktree",
].join("\n"),
);
const globalConfigPath = join(tmpDir, "global-agent-orchestrator.yaml");
writeFileSync(
globalConfigPath,
[
"defaults:",
" runtime: process",
" workspace: worktree",
" agent: claude-code",
" notifiers: []",
"projects:",
" agent-orchestrator_48321dec7a:",
` path: ${repoDir}`,
" sessionPrefix: current",
" defaultBranch: main",
].join("\n"),
);

mockConfigRef.current = makeConfig({
"agent-orchestrator_48321dec7a": makeProject({
name: "Current",
path: repoDir,
sessionPrefix: "current",
}),
});
(mockConfigRef.current as Record<string, unknown>).configPath = flatConfigPath;

const origEnv = process.env["AO_CONFIG_PATH"];
const origGlobalEnv = process.env["AO_GLOBAL_CONFIG"];
process.env["AO_CONFIG_PATH"] = flatConfigPath;
process.env["AO_GLOBAL_CONFIG"] = globalConfigPath;

const detectAgent = await import("../../src/lib/detect-agent.js");
vi.mocked(detectAgent.detectAvailableAgents).mockResolvedValue([
{ name: "codex", displayName: "Codex" },
{ name: "opencode", displayName: "OpenCode" },
]);
mockPromptSelect.mockResolvedValueOnce("codex").mockResolvedValueOnce("opencode");
const originalStdinTty = process.stdin.isTTY;
const originalStdoutTty = process.stdout.isTTY;
Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true });
Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true });

try {
await program.parseAsync([
"node",
"test",
"start",
"--interactive",
"--no-dashboard",
"--no-orchestrator",
]);

const flatAfter = parseYaml(readFileSync(flatConfigPath, "utf-8")) as {
projects?: unknown;
orchestrator?: { agent?: string };
worker?: { agent?: string };
path?: string;
};
expect(flatAfter.projects).toBeUndefined();
expect(flatAfter.path).toBeUndefined();
expect(flatAfter.orchestrator).toMatchObject({
agent: "codex",
});
expect(flatAfter.worker).toMatchObject({
agent: "opencode",
});
} finally {
Object.defineProperty(process.stdin, "isTTY", {
value: originalStdinTty,
configurable: true,
});
Object.defineProperty(process.stdout, "isTTY", {
value: originalStdoutTty,
configurable: true,
});
if (origEnv === undefined) delete process.env["AO_CONFIG_PATH"];
else process.env["AO_CONFIG_PATH"] = origEnv;
if (origGlobalEnv === undefined) delete process.env["AO_GLOBAL_CONFIG"];
else process.env["AO_GLOBAL_CONFIG"] = origGlobalEnv;
}
});
});
75 changes: 63 additions & 12 deletions packages/cli/src/commands/start.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,39 @@ function readProjectBehaviorConfig(projectPath: string): LocalProjectConfig {
return {};
}

function writeProjectBehaviorConfig(projectPath: string, config: LocalProjectConfig): void {
writeLocalProjectConfig(projectPath, config);
function isRecord(value: unknown): value is Record<string, unknown> {
return value !== null && typeof value === "object" && !Array.isArray(value);
}

function ensureRecord(value: unknown): Record<string, unknown> {
return isRecord(value) ? value : {};
}

function ensureWrappedProjectConfigContainer(
rawConfig: Record<string, unknown>,
): Record<string, Record<string, unknown>> {
if (!isRecord(rawConfig.projects)) {
rawConfig.projects = {};
}
return rawConfig.projects as Record<string, Record<string, unknown>>;
Comment thread
yyovil marked this conversation as resolved.
}

function getProjectConfig(
projects: Record<string, Record<string, unknown>>,
projectId: string,
fallback: ProjectConfig,
): Record<string, unknown> {
const projectConfig = projects[projectId];
if (isRecord(projectConfig)) return projectConfig;
return { ...(fallback as unknown as Record<string, unknown>) };
}

function writeProjectBehaviorConfig(
projectPath: string,
config: LocalProjectConfig,
configPath?: string,
): void {
writeLocalProjectConfig(projectPath, config, configPath);
}

/**
Expand Down Expand Up @@ -1650,11 +1681,13 @@ export function registerStart(program: Command): void {
// ── Handle "new orchestrator" choice (deferred from already-running check) ──
if (startNewOrchestrator) {
const rawYaml = readFileSync(config.configPath, "utf-8");
const rawConfig = yamlParse(rawYaml);
const rawConfig = ensureRecord(yamlParse(rawYaml));
Comment thread
yyovil marked this conversation as resolved.
const projects = ensureWrappedProjectConfigContainer(rawConfig);
const projectForNewOrchestrator = getProjectConfig(projects, projectId, project);

// Collect existing prefixes to avoid collisions
const existingPrefixes = new Set(
Object.values(rawConfig.projects as Record<string, Record<string, unknown>>)
Object.values(projects)
.map((p) => p.sessionPrefix as string)
.filter(Boolean),
);
Expand All @@ -1665,10 +1698,10 @@ export function registerStart(program: Command): void {
const suffix = Math.random().toString(36).slice(2, 6);
newId = `${projectId}-${suffix}`;
newPrefix = generateSessionPrefix(newId);
} while (rawConfig.projects[newId] || existingPrefixes.has(newPrefix));
} while (projects[newId] || existingPrefixes.has(newPrefix));

rawConfig.projects[newId] = {
...rawConfig.projects[projectId],
projects[newId] = {
...(projectForNewOrchestrator as Record<string, unknown>),
sessionPrefix: newPrefix,
};
const nextYaml = isCanonicalGlobalConfigPath(config.configPath)
Expand Down Expand Up @@ -1732,11 +1765,29 @@ export function registerStart(program: Command): void {
console.log(chalk.dim(` ✓ Saved to ${project.path}/agent-orchestrator.yaml\n`));
} else {
const rawYaml = readFileSync(config.configPath, "utf-8");
const rawConfig = yamlParse(rawYaml);
const proj = rawConfig.projects[projectId];
proj.orchestrator = { ...(proj.orchestrator ?? {}), agent: orchestratorAgent };
proj.worker = { ...(proj.worker ?? {}), agent: workerAgent };
writeFileSync(config.configPath, configToYaml(rawConfig as Record<string, unknown>));
const rawConfig = ensureRecord(yamlParse(rawYaml));
if ("projects" in rawConfig) {
const projects = ensureWrappedProjectConfigContainer(rawConfig);
const proj = getProjectConfig(projects, projectId, project);
proj.orchestrator = { ...(proj.orchestrator ?? {}), agent: orchestratorAgent };
proj.worker = { ...(proj.worker ?? {}), agent: workerAgent };
projects[projectId] = proj;
writeFileSync(
config.configPath,
configToYaml(rawConfig as Record<string, unknown>),
);
} else {
const nextLocalConfig = rawConfig as LocalProjectConfig;
nextLocalConfig.orchestrator = {
...(nextLocalConfig.orchestrator ?? {}),
agent: orchestratorAgent,
};
nextLocalConfig.worker = {
...(nextLocalConfig.worker ?? {}),
agent: workerAgent,
};
writeProjectBehaviorConfig(project.path, nextLocalConfig, config.configPath);
}
console.log(chalk.dim(` ✓ Saved to ${config.configPath}\n`));
}
config = loadConfig(config.configPath);
Expand Down
Loading