Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit 3b52838

Browse files
authored
Merge pull request #140 from closedloop-ai/FEA-763
FEA-763: Persist onboarding profile and filter Codex diagnostics
2 parents 8430cf1 + f50b268 commit 3b52838

6 files changed

Lines changed: 239 additions & 25 deletions

File tree

apps/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "desktop",
3-
"version": "0.14.7",
3+
"version": "0.14.8",
44
"description": "ClosedLoop Desktop",
55
"author": "ClosedLoop AI <support@closedloop.ai>",
66
"private": true,

apps/desktop/src/main/app.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -800,18 +800,7 @@ export class DesktopApplication {
800800
if (this.shouldStopManagedOnboardingRun(run, "managed key persistence")) {
801801
return;
802802
}
803-
this.apiKeyStore.setApiKey(claimResult.apiKey, "DESKTOP_MANAGED");
804-
this.persistActiveProfileKey(claimResult.apiKey, "DESKTOP_MANAGED");
805803
const keyPair = this.gatewaySigningKeyStore.load(activeGatewayId);
806-
this.persistActiveConfigManagedMetadata({
807-
apiKeySource: "DESKTOP_MANAGED",
808-
gatewayId: activeGatewayId,
809-
...(keyPair.ok
810-
? { gatewayPublicKeyPem: keyPair.keyPair.publicKeySpkiPem }
811-
: {}),
812-
desktopSecurityUpgradeProtocolVersion: 1,
813-
pendingOnboardingAttemptId: null,
814-
});
815804
const sandboxBaseDirectory = normalizeScopePath(
816805
payload.sandboxBaseDirectory,
817806
);
@@ -820,6 +809,7 @@ export class DesktopApplication {
820809
? sandboxBaseDirectory
821810
: null;
822811

812+
this.apiKeyStore.setApiKey(claimResult.apiKey, "DESKTOP_MANAGED");
823813
this.settingsStore.update({
824814
apiOrigin: trustedConfig.config.apiOrigin,
825815
relayOrigin: trustedConfig.config.relayOrigin,
@@ -831,10 +821,21 @@ export class DesktopApplication {
831821
}
832822
: { onboardingCompleted: false }),
833823
});
834-
this.settingsStore.updateActiveConfigOrigins({
835-
apiOrigin: trustedConfig.config.apiOrigin,
836-
relayOrigin: trustedConfig.config.relayOrigin,
837-
webAppOrigin: payload.webAppOrigin,
824+
const activeConfig =
825+
this.settingsStore.ensureActiveConfigForCurrentOrigins();
826+
this.apiKeyStore.saveProfileKey(
827+
activeConfig.id,
828+
claimResult.apiKey,
829+
"DESKTOP_MANAGED",
830+
);
831+
this.settingsStore.updateConfigManagedMetadata(activeConfig.id, {
832+
apiKeySource: "DESKTOP_MANAGED",
833+
gatewayId: activeGatewayId,
834+
...(keyPair.ok
835+
? { gatewayPublicKeyPem: keyPair.keyPair.publicKeySpkiPem }
836+
: {}),
837+
desktopSecurityUpgradeProtocolVersion: 1,
838+
pendingOnboardingAttemptId: null,
838839
});
839840

840841
if (safeSandboxBaseDirectory) {

apps/desktop/src/main/settings-store.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type SavedConfigOriginsPatch = Pick<
3434
"relayOrigin" | "apiOrigin" | "webAppOrigin"
3535
>;
3636

37+
const DEFAULT_MANAGED_ONBOARDING_CONFIG_NAME = "Default";
3738
const UUID_V4_RE =
3839
/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
3940

@@ -283,6 +284,62 @@ export class SettingsStore {
283284
);
284285
}
285286

287+
private getAvailableConfigName(preferredName: string): string {
288+
const baseName = this.validateConfigName(preferredName);
289+
const usedNames = new Set(
290+
this.getSavedConfigs().map((config) =>
291+
config.name.trim().toLocaleLowerCase(),
292+
),
293+
);
294+
if (!usedNames.has(baseName.toLocaleLowerCase())) {
295+
return baseName;
296+
}
297+
for (let suffix = 2; suffix < 1000; suffix += 1) {
298+
const candidate = `${baseName} ${suffix}`;
299+
if (!usedNames.has(candidate.toLocaleLowerCase())) {
300+
return candidate;
301+
}
302+
}
303+
throw new Error(`No available config name for "${baseName}"`);
304+
}
305+
306+
/**
307+
* Ensures the current runtime origins are represented by an active saved
308+
* profile, reusing a matching profile before creating a default one.
309+
*/
310+
ensureActiveConfigForCurrentOrigins(
311+
preferredName = DEFAULT_MANAGED_ONBOARDING_CONFIG_NAME,
312+
): SavedConfig {
313+
const relayOrigin = this.getRelayOrigin();
314+
const apiOrigin = this.getApiOrigin();
315+
const webAppOrigin = this.getWebAppOrigin();
316+
317+
const activeConfig = this.getActiveConfig();
318+
if (activeConfig) {
319+
return (
320+
this.updateActiveConfigOrigins({
321+
relayOrigin,
322+
apiOrigin,
323+
webAppOrigin,
324+
}) ?? activeConfig
325+
);
326+
}
327+
328+
const matchingConfig = this.findConfigByOrigins(
329+
relayOrigin,
330+
apiOrigin,
331+
webAppOrigin,
332+
);
333+
if (matchingConfig) {
334+
return this.applyConfig(matchingConfig.id);
335+
}
336+
337+
const savedConfig = this.saveConfig(
338+
this.getAvailableConfigName(preferredName),
339+
);
340+
return this.applyConfig(savedConfig.id);
341+
}
342+
286343
saveConfig(name: string): SavedConfig {
287344
const trimmedName = this.validateConfigName(name);
288345
const configs = this.getSavedConfigs();

apps/desktop/src/server/operations/codex.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { assertRepoAllowed, ensureWorktreeForReview, resolveWorktreeDir, resolve
1414
import { json } from "./response-utils.js";
1515

1616
const CODEX_SESSION_ID_REGEX = /session id:\s*([0-9a-f-]{36})/i;
17+
const CODEX_ROLLOUT_ITEM_RECORDING_DIAGNOSTIC_REGEX = /^\d{4}-\d{2}-\d{2}T[^\s]+\s+ERROR\s+codex_core::session:\s+failed to record rollout items:\s+thread\s+[0-9a-f-]{36}\s+not found$/i;
1718
const FINDINGS_CODE_BLOCK_REGEX = /```json\s*\n([\s\S]*?)\n\s*```/;
1819
const FINDINGS_ARRAY_REGEX = /\[[\s\S]*\]/;
1920
const PR_PREFIX_REGEX = /^pr-/;
@@ -161,17 +162,37 @@ export function extractTextFromNdjsonLog(raw: string, truncated = false): string
161162
try {
162163
const event = JSON.parse(line) as { type?: string; content?: string; error?: string };
163164
if (event.type === "text" && typeof event.content === "string") {
164-
parts.push(event.content);
165+
const content = stripCodexNonUserDiagnostics(event.content);
166+
if (content) {
167+
parts.push(content);
168+
}
165169
} else if (event.type === "error" && typeof event.error === "string") {
166-
parts.push(event.error);
170+
const error = stripCodexNonUserDiagnostics(event.error);
171+
if (error) {
172+
parts.push(error);
173+
}
167174
}
168175
} catch {
169-
parts.push(line);
176+
const content = stripCodexNonUserDiagnostics(line);
177+
if (content) {
178+
parts.push(content);
179+
}
170180
}
171181
}
172182
return parts.join("");
173183
}
174184

185+
/**
186+
* Removes Codex CLI diagnostics that describe local rollout recording failures,
187+
* not model review output or actionable user-facing failures.
188+
*/
189+
export function stripCodexNonUserDiagnostics(text: string): string {
190+
return text
191+
.split("\n")
192+
.filter((line) => !CODEX_ROLLOUT_ITEM_RECORDING_DIAGNOSTIC_REGEX.test(line.trim()))
193+
.join("\n");
194+
}
195+
175196
function tryKillRunningReview(state: ReviewState): void {
176197
if (state.status === "running" && state.pid) {
177198
try {
@@ -848,8 +869,9 @@ export function registerCodexRoutes(
848869
// Detect context window exhaustion — codex exited mid-review, findings are incomplete
849870
const isContextError = exitCode !== 0 && /context window|out of room/i.test(stderrHolder.value);
850871

851-
if (exitCode !== 0 && !isContextError && stderrHolder.value.trim()) {
852-
writeEvent(context.response, { type: "error", error: stderrHolder.value.trim() });
872+
const stderrForUser = stripCodexNonUserDiagnostics(stderrHolder.value).trim();
873+
if (exitCode !== 0 && !isContextError && stderrForUser) {
874+
writeEvent(context.response, { type: "error", error: stderrForUser });
853875
}
854876

855877
const finalState: ReviewState = {
@@ -1764,7 +1786,8 @@ async function streamClaudeReview(
17641786
});
17651787
}
17661788

1767-
function streamCodexReview(
1789+
/** @internal Streams Codex review stdout to SSE while retaining stderr for diagnostics. */
1790+
export function streamCodexReview(
17681791
child: ChildProcess,
17691792
response: ServerResponse,
17701793
logPath: string,
@@ -1797,8 +1820,6 @@ function streamCodexReview(
17971820
const text = typeof chunk === "string" ? chunk : chunk.toString("utf-8");
17981821
logStream.write(text);
17991822
stderrHolder.value += text;
1800-
eventCount++;
1801-
writeEvent(response, { type: "text", content: text });
18021823
});
18031824

18041825
child.on("close", () => {

apps/desktop/test/codex-log-parsing.test.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,14 @@ import fs from "node:fs/promises";
44
import os from "node:os";
55
import path from "node:path";
66
import { spawn } from "node:child_process";
7+
import type { ServerResponse } from "node:http";
78
import { afterEach, describe, test } from "node:test";
8-
import { extractTextFromNdjsonLog, extractVerdictTag } from "../src/server/operations/codex.js";
9+
import {
10+
extractTextFromNdjsonLog,
11+
extractVerdictTag,
12+
streamCodexReview,
13+
stripCodexNonUserDiagnostics,
14+
} from "../src/server/operations/codex.js";
915
import { createStreamState, processStreamEvent } from "../src/server/operations/stream-events.js";
1016

1117
const tempPaths: string[] = [];
@@ -102,6 +108,20 @@ describe("extractTextFromNdjsonLog", () => {
102108

103109
assert.equal(extractTextFromNdjsonLog(raw), "");
104110
});
111+
112+
test("removes Codex rollout recorder diagnostics from stored log text", () => {
113+
const diagnostic = "2026-04-30T15:21:30.628333Z ERROR codex_core::session: failed to record rollout items: thread 019ddefa-3be6-7b32-a969-c0f364fb225c not found";
114+
const raw = [
115+
"Finding before diagnostic.",
116+
diagnostic,
117+
JSON.stringify({ type: "text", content: `Finding after diagnostic.\n${diagnostic}\n` }),
118+
].join("\n");
119+
120+
assert.equal(
121+
extractTextFromNdjsonLog(raw),
122+
"Finding before diagnostic.Finding after diagnostic.\n"
123+
);
124+
});
105125
});
106126

107127
// ---------------------------------------------------------------------------
@@ -295,6 +315,49 @@ describe("streamClaudeReview log write ordering", () => {
295315
// ---------------------------------------------------------------------------
296316

297317
describe("streamCodexReview flush-gate pattern", () => {
318+
test("does not emit Codex stderr diagnostics as review text", async () => {
319+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "codex-stderr-diagnostic-"));
320+
tempPaths.push(tmpDir);
321+
const logPath = path.join(tmpDir, "codex-review.log");
322+
const diagnostic = "2026-04-30T15:21:30.714121Z ERROR codex_core::session: failed to record rollout items: thread 019ddefa-3a0e-78e2-90fa-c7c3e7bcc247 not found";
323+
324+
const child = spawn("node", [
325+
"-e",
326+
`process.stdout.write("review finding\\n"); process.stderr.write(${JSON.stringify(`${diagnostic}\n`)});`,
327+
]);
328+
329+
const events: Array<Record<string, unknown>> = [];
330+
const response = {
331+
destroyed: false,
332+
writable: true,
333+
write: (payload: string) => {
334+
events.push(JSON.parse(payload) as Record<string, unknown>);
335+
return true;
336+
},
337+
} as unknown as ServerResponse;
338+
const stderrHolder = { value: "" };
339+
340+
await streamCodexReview(child, response, logPath, { value: undefined }, stderrHolder);
341+
342+
assert.deepEqual(
343+
events.filter((event) => event.type === "text").map((event) => event.content),
344+
["review finding\n"]
345+
);
346+
assert.equal(events.some((event) => String(event.content ?? "").includes("failed to record rollout items")), false);
347+
assert.match(stderrHolder.value, /failed to record rollout items/);
348+
349+
const logContent = await fs.readFile(logPath, "utf-8");
350+
assert.match(logContent, /review finding/);
351+
assert.match(logContent, /failed to record rollout items/);
352+
});
353+
354+
test("stripCodexNonUserDiagnostics preserves real stderr failures", () => {
355+
const diagnostic = "2026-04-30T15:21:30.628333Z ERROR codex_core::session: failed to record rollout items: thread 019ddefa-3be6-7b32-a969-c0f364fb225c not found";
356+
const raw = `${diagnostic}\nreal failure\n${diagnostic}\n`;
357+
358+
assert.equal(stripCodexNonUserDiagnostics(raw), "real failure\n");
359+
});
360+
298361
test("log finish resolves only after all data is flushed from child stdout", async () => {
299362
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "codex-flush-gate-"));
300363
tempPaths.push(tmpDir);

apps/desktop/test/saved-configs.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,78 @@ test("findConfigByOrigins returns the matching config or null", () => {
131131
);
132132
});
133133

134+
test("ensureActiveConfigForCurrentOrigins creates and activates a default profile", () => {
135+
const tmpDir = makeTempDir("saved-configs-ensure-active-");
136+
const store = makeSettings(tmpDir);
137+
store.setRelayOrigin("https://relay.dev.test");
138+
store.setApiOrigin("https://api.dev.test");
139+
store.setWebAppOrigin("https://app.dev.test");
140+
141+
const config = store.ensureActiveConfigForCurrentOrigins("Default");
142+
143+
assert.equal(config.name, "Default");
144+
assert.equal(config.relayOrigin, "https://relay.dev.test");
145+
assert.equal(config.apiOrigin, "https://api.dev.test");
146+
assert.equal(config.webAppOrigin, "https://app.dev.test");
147+
assert.equal(store.getActiveConfigId(), config.id);
148+
assert.equal(store.listConfigs().length, 1);
149+
});
150+
151+
test("ensureActiveConfigForCurrentOrigins syncs origins onto the active profile", () => {
152+
const tmpDir = makeTempDir("saved-configs-ensure-active-origins-");
153+
const store = makeSettings(tmpDir);
154+
store.setRelayOrigin("https://relay.old.test");
155+
store.setApiOrigin("https://api.old.test");
156+
store.setWebAppOrigin("https://app.old.test");
157+
const existing = store.saveConfig("Development");
158+
store.applyConfig(existing.id);
159+
store.setRelayOrigin("https://relay.new.test");
160+
store.setApiOrigin("https://api.new.test");
161+
store.setWebAppOrigin("https://app.new.test");
162+
163+
const config = store.ensureActiveConfigForCurrentOrigins("Default");
164+
165+
assert.equal(config.id, existing.id);
166+
assert.equal(config.relayOrigin, "https://relay.new.test");
167+
assert.equal(config.apiOrigin, "https://api.new.test");
168+
assert.equal(config.webAppOrigin, "https://app.new.test");
169+
assert.equal(store.listConfigs().length, 1);
170+
});
171+
172+
test("ensureActiveConfigForCurrentOrigins reuses a matching saved profile", () => {
173+
const tmpDir = makeTempDir("saved-configs-ensure-reuse-");
174+
const store = makeSettings(tmpDir);
175+
store.setRelayOrigin("https://relay.dev.test");
176+
store.setApiOrigin("https://api.dev.test");
177+
store.setWebAppOrigin("https://app.dev.test");
178+
const existing = store.saveConfig("Development");
179+
180+
const config = store.ensureActiveConfigForCurrentOrigins("Default");
181+
182+
assert.equal(config.id, existing.id);
183+
assert.equal(config.name, "Development");
184+
assert.equal(store.getActiveConfigId(), existing.id);
185+
assert.equal(store.listConfigs().length, 1);
186+
});
187+
188+
test("ensureActiveConfigForCurrentOrigins chooses an available default name", () => {
189+
const tmpDir = makeTempDir("saved-configs-ensure-name-");
190+
const store = makeSettings(tmpDir);
191+
store.setRelayOrigin("https://relay.one.test");
192+
store.setApiOrigin("https://api.one.test");
193+
store.setWebAppOrigin("https://app.one.test");
194+
store.saveConfig("Default");
195+
store.setRelayOrigin("https://relay.two.test");
196+
store.setApiOrigin("https://api.two.test");
197+
store.setWebAppOrigin("https://app.two.test");
198+
199+
const config = store.ensureActiveConfigForCurrentOrigins("Default");
200+
201+
assert.equal(config.name, "Default 2");
202+
assert.equal(store.getActiveConfigId(), config.id);
203+
assert.equal(store.listConfigs().length, 2);
204+
});
205+
134206
// --- listConfigs ---
135207

136208
test("listConfigs returns configs in insertion order", () => {

0 commit comments

Comments
 (0)