chore: improve storage/ implementation; use log(...) consistently (#473)

This PR tidies up primitives under storage/.

**Noop changes:**

* Promote logger implementation to top-level utility outside of agent/
* Use logger within storage primitives
* Cleanup doc strings and comments

**Functional changes:**

* Increase command history size to 10_000
* Remove unnecessary debounce implementation and ensure a session ID is
created only once per agent loop

---------

Signed-off-by: Thibault Sottiaux <tibo@openai.com>
This commit is contained in:
Thibault Sottiaux
2025-04-21 09:51:34 -04:00
committed by GitHub
parent 8f1ea7fa85
commit dc276999a9
15 changed files with 49 additions and 76 deletions

View File

@@ -14,7 +14,6 @@ import type { ResponseItem } from "openai/resources/responses/responses";
import App from "./app"; import App from "./app";
import { runSinglePass } from "./cli-singlepass"; import { runSinglePass } from "./cli-singlepass";
import { AgentLoop } from "./utils/agent/agent-loop"; import { AgentLoop } from "./utils/agent/agent-loop";
import { initLogger } from "./utils/agent/log";
import { ReviewDecision } from "./utils/agent/review"; import { ReviewDecision } from "./utils/agent/review";
import { AutoApprovalMode } from "./utils/auto-approval-mode"; import { AutoApprovalMode } from "./utils/auto-approval-mode";
import { checkForUpdates } from "./utils/check-updates"; import { checkForUpdates } from "./utils/check-updates";
@@ -25,6 +24,7 @@ import {
INSTRUCTIONS_FILEPATH, INSTRUCTIONS_FILEPATH,
} from "./utils/config"; } from "./utils/config";
import { createInputItem } from "./utils/input-utils"; import { createInputItem } from "./utils/input-utils";
import { initLogger } from "./utils/logger/log";
import { isModelSupportedForResponses } from "./utils/model-utils.js"; import { isModelSupportedForResponses } from "./utils/model-utils.js";
import { parseToolCall } from "./utils/parsers"; import { parseToolCall } from "./utils/parsers";
import { onExit, setInkRenderer } from "./utils/terminal"; import { onExit, setInkRenderer } from "./utils/terminal";

View File

@@ -1,4 +1,4 @@
import { log } from "../../utils/agent/log.js"; import { log } from "../../utils/logger/log.js";
import { Box, Text, useInput, useStdin } from "ink"; import { Box, Text, useInput, useStdin } from "ink";
import React, { useState } from "react"; import React, { useState } from "react";
import { useInterval } from "use-interval"; import { useInterval } from "use-interval";

View File

@@ -9,10 +9,10 @@ import type {
import MultilineTextEditor from "./multiline-editor"; import MultilineTextEditor from "./multiline-editor";
import { TerminalChatCommandReview } from "./terminal-chat-command-review.js"; import { TerminalChatCommandReview } from "./terminal-chat-command-review.js";
import TextCompletions from "./terminal-chat-completions.js"; import TextCompletions from "./terminal-chat-completions.js";
import { log } from "../../utils/agent/log.js";
import { loadConfig } from "../../utils/config.js"; import { loadConfig } from "../../utils/config.js";
import { getFileSystemSuggestions } from "../../utils/file-system-suggestions.js"; import { getFileSystemSuggestions } from "../../utils/file-system-suggestions.js";
import { createInputItem } from "../../utils/input-utils.js"; import { createInputItem } from "../../utils/input-utils.js";
import { log } from "../../utils/logger/log.js";
import { setSessionId } from "../../utils/session.js"; import { setSessionId } from "../../utils/session.js";
import { SLASH_COMMANDS, type SlashCommand } from "../../utils/slash-commands"; import { SLASH_COMMANDS, type SlashCommand } from "../../utils/slash-commands";
import { import {

View File

@@ -8,9 +8,9 @@ import type {
import MultilineTextEditor from "./multiline-editor"; import MultilineTextEditor from "./multiline-editor";
import { TerminalChatCommandReview } from "./terminal-chat-command-review.js"; import { TerminalChatCommandReview } from "./terminal-chat-command-review.js";
import { log } from "../../utils/agent/log.js";
import { loadConfig } from "../../utils/config.js"; import { loadConfig } from "../../utils/config.js";
import { createInputItem } from "../../utils/input-utils.js"; import { createInputItem } from "../../utils/input-utils.js";
import { log } from "../../utils/logger/log.js";
import { setSessionId } from "../../utils/session.js"; import { setSessionId } from "../../utils/session.js";
import { import {
loadCommandHistory, loadCommandHistory,

View File

@@ -15,13 +15,13 @@ import { formatCommandForDisplay } from "../../format-command.js";
import { useConfirmation } from "../../hooks/use-confirmation.js"; import { useConfirmation } from "../../hooks/use-confirmation.js";
import { useTerminalSize } from "../../hooks/use-terminal-size.js"; import { useTerminalSize } from "../../hooks/use-terminal-size.js";
import { AgentLoop } from "../../utils/agent/agent-loop.js"; import { AgentLoop } from "../../utils/agent/agent-loop.js";
import { log } from "../../utils/agent/log.js";
import { ReviewDecision } from "../../utils/agent/review.js"; import { ReviewDecision } from "../../utils/agent/review.js";
import { generateCompactSummary } from "../../utils/compact-summary.js"; import { generateCompactSummary } from "../../utils/compact-summary.js";
import { OPENAI_BASE_URL, saveConfig } from "../../utils/config.js"; import { OPENAI_BASE_URL, saveConfig } from "../../utils/config.js";
import { extractAppliedPatches as _extractAppliedPatches } from "../../utils/extract-applied-patches.js"; import { extractAppliedPatches as _extractAppliedPatches } from "../../utils/extract-applied-patches.js";
import { getGitDiff } from "../../utils/get-diff.js"; import { getGitDiff } from "../../utils/get-diff.js";
import { createInputItem } from "../../utils/input-utils.js"; import { createInputItem } from "../../utils/input-utils.js";
import { log } from "../../utils/logger/log.js";
import { getAvailableModels } from "../../utils/model-utils.js"; import { getAvailableModels } from "../../utils/model-utils.js";
import { CLI_VERSION } from "../../utils/session.js"; import { CLI_VERSION } from "../../utils/session.js";
import { shortCwd } from "../../utils/short-path.js"; import { shortCwd } from "../../utils/short-path.js";
@@ -237,6 +237,7 @@ export default function TerminalChat({
// Tear down any existing loop before creating a new one // Tear down any existing loop before creating a new one
agentRef.current?.terminate(); agentRef.current?.terminate();
const sessionId = crypto.randomUUID();
agentRef.current = new AgentLoop({ agentRef.current = new AgentLoop({
model, model,
provider, provider,
@@ -249,7 +250,7 @@ export default function TerminalChat({
log(`onItem: ${JSON.stringify(item)}`); log(`onItem: ${JSON.stringify(item)}`);
setItems((prev) => { setItems((prev) => {
const updated = uniqueById([...prev, item as ResponseItem]); const updated = uniqueById([...prev, item as ResponseItem]);
saveRollout(updated); saveRollout(sessionId, updated);
return updated; return updated;
}); });
}, },

View File

@@ -10,8 +10,8 @@ import type {
} from "openai/resources/responses/responses.mjs"; } from "openai/resources/responses/responses.mjs";
import type { Reasoning } from "openai/resources.mjs"; import type { Reasoning } from "openai/resources.mjs";
import { log } from "./log.js";
import { OPENAI_TIMEOUT_MS, getApiKey, getBaseUrl } from "../config.js"; import { OPENAI_TIMEOUT_MS, getApiKey, getBaseUrl } from "../config.js";
import { log } from "../logger/log.js";
import { parseToolCallArguments } from "../parsers.js"; import { parseToolCallArguments } from "../parsers.js";
import { responsesCreateViaChatCompletions } from "../responses.js"; import { responsesCreateViaChatCompletions } from "../responses.js";
import { import {

View File

@@ -5,12 +5,12 @@ import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js";
import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs";
import { exec, execApplyPatch } from "./exec.js"; import { exec, execApplyPatch } from "./exec.js";
import { isLoggingEnabled, log } from "./log.js";
import { ReviewDecision } from "./review.js"; import { ReviewDecision } from "./review.js";
import { FullAutoErrorMode } from "../auto-approval-mode.js"; import { FullAutoErrorMode } from "../auto-approval-mode.js";
import { SandboxType } from "./sandbox/interface.js"; import { SandboxType } from "./sandbox/interface.js";
import { canAutoApprove } from "../../approvals.js"; import { canAutoApprove } from "../../approvals.js";
import { formatCommandForDisplay } from "../../format-command.js"; import { formatCommandForDisplay } from "../../format-command.js";
import { isLoggingEnabled, log } from "../logger/log.js";
import { access } from "fs/promises"; import { access } from "fs/promises";
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@@ -2,7 +2,7 @@
* Utility functions for handling platform-specific commands * Utility functions for handling platform-specific commands
*/ */
import { log } from "./log.js"; import { log } from "../logger/log.js";
/** /**
* Map of Unix commands to their Windows equivalents * Map of Unix commands to their Windows equivalents

View File

@@ -2,7 +2,7 @@ import type { ExecResult } from "./interface.js";
import type { SpawnOptions } from "child_process"; import type { SpawnOptions } from "child_process";
import { exec } from "./raw-exec.js"; import { exec } from "./raw-exec.js";
import { log } from "../log.js"; import { log } from "../../logger/log.js";
function getCommonRoots() { function getCommonRoots() {
return [ return [

View File

@@ -7,7 +7,7 @@ import type {
StdioPipe, StdioPipe,
} from "child_process"; } from "child_process";
import { log } from "../log.js"; import { log } from "../../logger/log.js";
import { adaptCommandForPlatform } from "../platform-commands.js"; import { adaptCommandForPlatform } from "../platform-commands.js";
import { spawn } from "child_process"; import { spawn } from "child_process";
import * as os from "os"; import * as os from "os";

View File

@@ -8,8 +8,8 @@
import type { FullAutoErrorMode } from "./auto-approval-mode.js"; import type { FullAutoErrorMode } from "./auto-approval-mode.js";
import { log } from "./agent/log.js";
import { AutoApprovalMode } from "./auto-approval-mode.js"; import { AutoApprovalMode } from "./auto-approval-mode.js";
import { log } from "./logger/log.js";
import { providers } from "./providers.js"; import { providers } from "./providers.js";
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
import { load as loadYaml, dump as dumpYaml } from "js-yaml"; import { load as loadYaml, dump as dumpYaml } from "js-yaml";

View File

@@ -1,12 +1,13 @@
import { log } from "../logger/log.js";
import { existsSync } from "fs"; import { existsSync } from "fs";
import fs from "fs/promises"; import fs from "fs/promises";
import os from "os"; import os from "os";
import path from "path"; import path from "path";
const HISTORY_FILE = path.join(os.homedir(), ".codex", "history.json"); const HISTORY_FILE = path.join(os.homedir(), ".codex", "history.json");
const DEFAULT_HISTORY_SIZE = 1000; const DEFAULT_HISTORY_SIZE = 10_000;
// Regex patterns for sensitive commands that should not be saved // Regex patterns for sensitive commands that should not be saved.
const SENSITIVE_PATTERNS = [ const SENSITIVE_PATTERNS = [
/\b[A-Za-z0-9-_]{20,}\b/, // API keys and tokens /\b[A-Za-z0-9-_]{20,}\b/, // API keys and tokens
/\bpassword\b/i, /\bpassword\b/i,
@@ -18,7 +19,7 @@ const SENSITIVE_PATTERNS = [
export interface HistoryConfig { export interface HistoryConfig {
maxSize: number; maxSize: number;
saveHistory: boolean; saveHistory: boolean;
sensitivePatterns: Array<string>; // Array of regex patterns as strings sensitivePatterns: Array<string>; // Regex patterns.
} }
export interface HistoryEntry { export interface HistoryEntry {
@@ -32,9 +33,6 @@ export const DEFAULT_HISTORY_CONFIG: HistoryConfig = {
sensitivePatterns: [], sensitivePatterns: [],
}; };
/**
* Loads command history from the history file
*/
export async function loadCommandHistory(): Promise<Array<HistoryEntry>> { export async function loadCommandHistory(): Promise<Array<HistoryEntry>> {
try { try {
if (!existsSync(HISTORY_FILE)) { if (!existsSync(HISTORY_FILE)) {
@@ -45,26 +43,21 @@ export async function loadCommandHistory(): Promise<Array<HistoryEntry>> {
const history = JSON.parse(data) as Array<HistoryEntry>; const history = JSON.parse(data) as Array<HistoryEntry>;
return Array.isArray(history) ? history : []; return Array.isArray(history) ? history : [];
} catch (error) { } catch (error) {
// Use error logger but for production would use a proper logging system log(`error: failed to load command history: ${error}`);
// eslint-disable-next-line no-console
console.error("Failed to load command history:", error);
return []; return [];
} }
} }
/**
* Saves command history to the history file
*/
export async function saveCommandHistory( export async function saveCommandHistory(
history: Array<HistoryEntry>, history: Array<HistoryEntry>,
config: HistoryConfig = DEFAULT_HISTORY_CONFIG, config: HistoryConfig = DEFAULT_HISTORY_CONFIG,
): Promise<void> { ): Promise<void> {
try { try {
// Create directory if it doesn't exist // Create directory if it doesn't exist.
const dir = path.dirname(HISTORY_FILE); const dir = path.dirname(HISTORY_FILE);
await fs.mkdir(dir, { recursive: true }); await fs.mkdir(dir, { recursive: true });
// Trim history to max size // Trim history to max size.
const trimmedHistory = history.slice(-config.maxSize); const trimmedHistory = history.slice(-config.maxSize);
await fs.writeFile( await fs.writeFile(
@@ -73,14 +66,10 @@ export async function saveCommandHistory(
"utf-8", "utf-8",
); );
} catch (error) { } catch (error) {
// eslint-disable-next-line no-console log(`error: failed to save command history: ${error}`);
console.error("Failed to save command history:", error);
} }
} }
/**
* Adds a command to history if it's not sensitive
*/
export async function addToHistory( export async function addToHistory(
command: string, command: string,
history: Array<HistoryEntry>, history: Array<HistoryEntry>,
@@ -90,46 +79,41 @@ export async function addToHistory(
return history; return history;
} }
// Check if command contains sensitive information // Skip commands with sensitive information.
if (isSensitiveCommand(command, config.sensitivePatterns)) { if (commandHasSensitiveInfo(command, config.sensitivePatterns)) {
return history; return history;
} }
// Check for duplicate (don't add if it's the same as the last command) // Check for duplicate (don't add if it's the same as the last command).
const lastEntry = history[history.length - 1]; const lastEntry = history[history.length - 1];
if (lastEntry && lastEntry.command === command) { if (lastEntry && lastEntry.command === command) {
return history; return history;
} }
// Add new entry // Add new entry.
const newEntry: HistoryEntry = { const newHistory: Array<HistoryEntry> = [
command, ...history,
timestamp: Date.now(), {
}; command,
timestamp: Date.now(),
const newHistory = [...history, newEntry]; },
];
// Save to file
await saveCommandHistory(newHistory, config); await saveCommandHistory(newHistory, config);
return newHistory; return newHistory;
} }
/** function commandHasSensitiveInfo(
* Checks if a command contains sensitive information
*/
function isSensitiveCommand(
command: string, command: string,
additionalPatterns: Array<string> = [], additionalPatterns: Array<string> = [],
): boolean { ): boolean {
// Check built-in patterns // Check built-in patterns.
for (const pattern of SENSITIVE_PATTERNS) { for (const pattern of SENSITIVE_PATTERNS) {
if (pattern.test(command)) { if (pattern.test(command)) {
return true; return true;
} }
} }
// Check additional patterns from config // Check additional patterns from config.
for (const patternStr of additionalPatterns) { for (const patternStr of additionalPatterns) {
try { try {
const pattern = new RegExp(patternStr); const pattern = new RegExp(patternStr);
@@ -137,23 +121,19 @@ function isSensitiveCommand(
return true; return true;
} }
} catch (error) { } catch (error) {
// Invalid regex pattern, skip it // Invalid regex pattern, skip it.
} }
} }
return false; return false;
} }
/**
* Clears the command history
*/
export async function clearCommandHistory(): Promise<void> { export async function clearCommandHistory(): Promise<void> {
try { try {
if (existsSync(HISTORY_FILE)) { if (existsSync(HISTORY_FILE)) {
await fs.writeFile(HISTORY_FILE, JSON.stringify([]), "utf-8"); await fs.writeFile(HISTORY_FILE, JSON.stringify([]), "utf-8");
} }
} catch (error) { } catch (error) {
// eslint-disable-next-line no-console log(`error: failed to clear command history: ${error}`);
console.error("Failed to clear command history:", error);
} }
} }

View File

@@ -1,20 +1,19 @@
/* eslint-disable no-console */
import type { ResponseItem } from "openai/resources/responses/responses"; import type { ResponseItem } from "openai/resources/responses/responses";
import { loadConfig } from "../config"; import { loadConfig } from "../config";
import { log } from "../logger/log.js";
import fs from "fs/promises"; import fs from "fs/promises";
import os from "os"; import os from "os";
import path from "path"; import path from "path";
const SESSIONS_ROOT = path.join(os.homedir(), ".codex", "sessions"); const SESSIONS_ROOT = path.join(os.homedir(), ".codex", "sessions");
async function saveRolloutToHomeSessions( async function saveRolloutAsync(
sessionId: string,
items: Array<ResponseItem>, items: Array<ResponseItem>,
): Promise<void> { ): Promise<void> {
await fs.mkdir(SESSIONS_ROOT, { recursive: true }); await fs.mkdir(SESSIONS_ROOT, { recursive: true });
const sessionId = crypto.randomUUID();
const timestamp = new Date().toISOString(); const timestamp = new Date().toISOString();
const ts = timestamp.replace(/[:.]/g, "-").slice(0, 10); const ts = timestamp.replace(/[:.]/g, "-").slice(0, 10);
const filename = `rollout-${ts}-${sessionId}.json`; const filename = `rollout-${ts}-${sessionId}.json`;
@@ -39,23 +38,15 @@ async function saveRolloutToHomeSessions(
"utf8", "utf8",
); );
} catch (error) { } catch (error) {
console.error(`Failed to save rollout to ${filePath}: `, error); log(`error: failed to save rollout to ${filePath}: ${error}`);
} }
} }
let debounceTimer: NodeJS.Timeout | null = null; export function saveRollout(
let pendingItems: Array<ResponseItem> | null = null; sessionId: string,
items: Array<ResponseItem>,
export function saveRollout(items: Array<ResponseItem>): void { ): void {
pendingItems = items; // Best-effort. We also do not log here in case of failure as that should be taken care of
if (debounceTimer) { // by `saveRolloutAsync` already.
clearTimeout(debounceTimer); saveRolloutAsync(sessionId, items).catch(() => {});
}
debounceTimer = setTimeout(() => {
if (pendingItems) {
saveRolloutToHomeSessions(pendingItems).catch(() => {});
pendingItems = null;
}
debounceTimer = null;
}, 2000);
} }

View File

@@ -9,6 +9,7 @@
"build": "pnpm --filter @openai/codex run build", "build": "pnpm --filter @openai/codex run build",
"test": "pnpm --filter @openai/codex run test", "test": "pnpm --filter @openai/codex run test",
"lint": "pnpm --filter @openai/codex run lint", "lint": "pnpm --filter @openai/codex run lint",
"lint:fix": "pnpm --filter @openai/codex run lint:fix",
"typecheck": "pnpm --filter @openai/codex run typecheck", "typecheck": "pnpm --filter @openai/codex run typecheck",
"changelog": "git-cliff --config cliff.toml --output CHANGELOG.ignore.md $LAST_RELEASE_TAG..HEAD", "changelog": "git-cliff --config cliff.toml --output CHANGELOG.ignore.md $LAST_RELEASE_TAG..HEAD",
"prepare": "husky", "prepare": "husky",