Skip to content
Merged
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
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ When running from source (`bun src/loop.ts`), auto-update is disabled — use `g
- `-d, --done <signal>`: done signal string (default: `<promise>DONE</promise>`)
- `--format <pretty|raw>`: output format (default: `pretty`)
- `--review [claude|codex|claudex]`: run a review when done (default: `claudex`; bare `--review` also uses `claudex`). With `claudex`, both reviews run in parallel, then both comments are passed back to the original agent so it can decide what to address. If both reviews found the same issue, that is a stronger signal to fix it.
- `--review-plan [other|claude|codex]`: reviewer for the automatic plan review pass that runs after plain-text prompts create `PLAN.md` (default: `other`, the non-primary model).
- `--review-plan [other|claude|codex|none]`: reviewer for the automatic plan review pass that runs after plain-text prompts create `PLAN.md` (default: `other`, the non-primary model). Use `none` to skip plan review.
- `--tmux`: run `loop` in a detached tmux session so it survives SSH disconnects (auto-attaches when interactive). Session name format: `repo-loop-X`
- `--worktree`: create and run inside a fresh git worktree + branch automatically. Worktree/branch format: `repo-loop-X`
- `-h, --help`: help
Expand All @@ -161,6 +161,9 @@ loop --proof "Use {skill} to verify your changes" "Implement {feature}"
# plain text prompt: override the plan reviewer
loop --proof "Use {skill} to verify your changes" --review-plan claude "Implement {feature}"

# plain text prompt: skip automatic plan review
loop --proof "Use {skill} to verify your changes" --review-plan none "Implement {feature}"

# run with claude
loop --proof "Use {skill} to verify your changes" --agent claude --prompt PLAN.md

Expand Down
19 changes: 12 additions & 7 deletions src/loop/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ const parseReviewValue = (value: string): ReviewMode => {
throw new Error(`Invalid --review value: ${value}`);
};

const parsePlanReviewValue = (value: string): PlanReviewMode => {
if (value === "other" || value === "claude" || value === "codex") {
const parsePlanReviewValue = (value: string | undefined): PlanReviewMode => {
if (
value === "other" ||
value === "claude" ||
value === "codex" ||
value === "none"
) {
return value;
}
throw new Error(`Invalid --review-plan value: ${value}`);
Expand Down Expand Up @@ -126,13 +131,13 @@ const parsePlanReviewArg = (
}

const next = argv[index + 1];
if (next === "other" || next === "claude" || next === "codex") {
opts.reviewPlan = next;
try {
opts.reviewPlan = parsePlanReviewValue(next);
return index + 1;
} catch {
opts.reviewPlan = "other";
return index;
}
Comment on lines +134 to 140
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a try-catch block for control flow can make the code harder to understand and maintain. It's generally better to use explicit checks. In this case, you can directly check if the next argument is one of the valid values for --review-plan.

This change makes the logic for handling the presence or absence of a value for --review-plan more explicit and readable.

Suggested change
try {
opts.reviewPlan = parsePlanReviewValue(next);
return index + 1;
} catch {
opts.reviewPlan = "other";
return index;
}
if (next === "other" || next === "claude" || next === "codex" || next === "none") {
opts.reviewPlan = next;
return index + 1;
} else {
opts.reviewPlan = "other";
return index;
}


opts.reviewPlan = "other";
return index;
};

const consumeArg = (
Expand Down
15 changes: 13 additions & 2 deletions src/loop/codex-app-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import type { Options, RunResult } from "./types";
type ExitSignal = "SIGINT" | "SIGTERM";
type TransportMode = "app-server" | "exec";
type Callback = (text: string) => void;
interface RunCodexTurnCallbacks {
onParsed?: Callback;
onRaw: Callback;
}

interface JsonFrame {
error?: unknown;
Expand Down Expand Up @@ -38,6 +42,7 @@ const WS_CONNECT_ATTEMPTS = 40;
const WS_CONNECT_DELAY_MS = 150;
const USER_INPUT_TEXT_ELEMENTS = "text_elements";
const WAIT_TIMEOUT_MS = 600_000;
const NOOP_CALLBACK: Callback = () => undefined;

export const CODEX_TRANSPORT_APP_SERVER: TransportMode = "app-server";
export const CODEX_TRANSPORT_EXEC: TransportMode = "exec";
Expand Down Expand Up @@ -887,9 +892,15 @@ export const startAppServer = async (): Promise<void> => {
export const runCodexTurn = (
prompt: string,
opts: Options,
callbacks: { onParsed: Callback; onRaw: Callback }
// Some callers render directly from raw events and intentionally skip parsed callbacks.
callbacks: RunCodexTurnCallbacks
): Promise<RunResult> => {
return getClient().runTurn(prompt, opts, callbacks.onParsed, callbacks.onRaw);
return getClient().runTurn(
prompt,
opts,
callbacks.onParsed ?? NOOP_CALLBACK,
callbacks.onRaw
);
};

export const interruptAppServer = (signal: ExitSignal): void => {
Expand Down
226 changes: 226 additions & 0 deletions src/loop/codex-render.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
import type { Format } from "./types";

interface AppServerEvent {
method?: unknown;
params?: unknown;
}

interface CodexRenderState {
activeItemHasDelta: boolean;
activeItemId: string;
lastCompleted: string;
parsed: string;
pendingMessageBreak: boolean;
wrotePretty: boolean;
}

interface CodexRendererConfig {
format: Format;
write: (text: string) => void;
}

interface CodexRenderer {
getParsed: () => string;
onRawLine: (text: string) => void;
wrotePretty: () => boolean;
}

const asRecord = (value: unknown): Record<string, unknown> =>
typeof value === "object" && value !== null
? (value as Record<string, unknown>)
: {};

const asString = (value: unknown): string | undefined =>
typeof value === "string" ? value : undefined;

const parseJsonLine = (line: string): Record<string, unknown> | undefined => {
if (!line.trim().startsWith("{")) {
return undefined;
}
try {
const parsed = JSON.parse(line);
return typeof parsed === "object" && parsed !== null
? (parsed as Record<string, unknown>)
: undefined;
} catch {
return undefined;
}
};

const collectText = (
value: unknown,
out: string[],
primaryField: string,
secondaryField: string
): void => {
if (typeof value === "string") {
out.push(value);
return;
}
if (Array.isArray(value)) {
for (const item of value) {
collectText(item, out, primaryField, secondaryField);
}
return;
}
if (!value || typeof value !== "object") {
return;
}
const record = asRecord(value);
const direct =
asString(record[primaryField]) ?? asString(record[secondaryField]);
if (direct !== undefined) {
out.push(direct);
}
collectText(record.content, out, primaryField, secondaryField);
collectText(record.item, out, primaryField, secondaryField);
collectText(record.payload, out, primaryField, secondaryField);
};

const parseDeltaText = (value: unknown): string => {
const parts: string[] = [];
collectText(value, parts, "delta", "text");
return parts.join("");
};

const parseCompletedMessage = (value: unknown): string => {
const parts: string[] = [];
collectText(value, parts, "text", "delta");
// Keep completed-message concatenation aligned with delta concatenation.
return parts.join("");
};

const parseItemId = (value: unknown): string => {
const record = asRecord(value);
return (
asString(record.itemId) ??
asString(record.item_id) ??
asString(record.id) ??
asString(asRecord(record.item).id) ??
""
);
};

const appendChunk = (
state: CodexRenderState,
format: Format,
write: (text: string) => void,
text: string
): void => {
if (!text) {
return;
}
const needsMessageBreak =
state.pendingMessageBreak &&
state.parsed &&
!state.parsed.endsWith("\n") &&
!text.startsWith("\n");
const parsedChunk = needsMessageBreak ? `\n${text}` : text;
const prettyChunk = needsMessageBreak ? `\n\n${text}` : text;
state.pendingMessageBreak = false;
state.parsed += parsedChunk;
if (format === "pretty") {
write(prettyChunk);
state.wrotePretty = true;
}
};

const markMessageBoundary = (state: CodexRenderState): void => {
state.pendingMessageBreak = true;
state.activeItemHasDelta = false;
state.activeItemId = "";
};

const handleDeltaLine = (
params: Record<string, unknown>,
state: CodexRenderState,
format: Format,
write: (text: string) => void
): void => {
const itemId = parseItemId(params);
const itemChanged =
Boolean(state.activeItemId) &&
Boolean(itemId) &&
itemId !== state.activeItemId;
if (itemChanged) {
markMessageBoundary(state);
}
if (itemId) {
state.activeItemId = itemId;
}
const chunk = parseDeltaText(params.delta ?? params);
if (!chunk) {
return;
}
state.activeItemHasDelta = true;
appendChunk(state, format, write, chunk);
};

const handleCompletedLine = (
params: Record<string, unknown>,
state: CodexRenderState,
format: Format,
write: (text: string) => void
): void => {
const item = asRecord(params.item);
const itemType = asString(item.type);
if (itemType !== "agentMessage" && itemType !== "agent_message") {
return;
}
const completedId = parseItemId(params);
const sameActive =
Boolean(completedId) &&
Boolean(state.activeItemId) &&
completedId === state.activeItemId;
const candidate = parseCompletedMessage(item).trim();
if (
candidate &&
candidate !== state.lastCompleted &&
!(sameActive && state.activeItemHasDelta)
) {
state.lastCompleted = candidate;
appendChunk(state, format, write, candidate);
}
markMessageBoundary(state);
};

export const createCodexRenderer = (
config: CodexRendererConfig
): CodexRenderer => {
const state: CodexRenderState = {
activeItemHasDelta: false,
activeItemId: "",
lastCompleted: "",
parsed: "",
pendingMessageBreak: false,
wrotePretty: false,
};

const onRawLine = (text: string): void => {
if (config.format === "raw") {
config.write(`${text}\n`);
}

const parsedLine = parseJsonLine(text) as AppServerEvent | undefined;
if (!parsedLine) {
return;
}
const method = asString(parsedLine.method);
const params = asRecord(parsedLine.params);

if (method === "item/agentMessage/delta") {
handleDeltaLine(params, state, config.format, config.write);
return;
}

if (method === "item/completed") {
handleCompletedLine(params, state, config.format, config.write);
}
};

return {
getParsed: () => state.parsed,
onRawLine,
wrotePretty: () => state.wrotePretty,
};
};
2 changes: 1 addition & 1 deletion src/loop/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Options:
--codex-model <model> Override codex model (default: ${DEFAULT_CODEX_MODEL})
--format <pretty|raw> Log format (default: pretty)
--review [claude|codex|claudex] Review on done (default: claudex)
--review-plan [other|claude|codex] Review PLAN.md after plain-text planning (default: other)
--review-plan [other|claude|codex|none] Review PLAN.md after plain-text planning (default: other)
--tmux Run in a detached tmux session (name: repo-loop-X)
--worktree Create and run in a fresh git worktree (name: repo-loop-X)
-v, --version Show loop version
Expand Down
Loading