Skip to content

Enable Codex structured output in Rig Codex adapter #283

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

Merged
pelikhan merged 1 commit into from
Jul 29, 2026
+40 −4
Merged

Enable Codex structured output in Rig Codex adapter #283

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
10 changes: 8 additions & 2 deletions skills/rig/engines/codex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,17 @@ export function codexEngine(options: CodexEngineOptions = {}): AgentFactory {
const signal = askOptions.signal
? AbortSignal.any([askOptions.signal, closeController.signal])
: closeController.signal;
const activeTurn = thread.run(prompt, { signal });
const activeTurn = thread.run(prompt, {
signal,
...(askOptions.outputSchema !== undefined && { outputSchema: askOptions.outputSchema }),
});
activeTurns.add(activeTurn);
try {
const turn = await activeTurn;
return turn.finalResponse;
if (typeof turn.finalResponse === "string") {
return turn.finalResponse;
}
return JSON.stringify(turn.finalResponse);

@github-actions github-actions Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] Missing edge case: finalResponse could be null — JSON.stringify(null) returns "null", which propagates silently as a valid response but will fail downstream schema validation.

💡 Suggested test + guard 
mocks.run.mockResolvedValueOnce({ finalResponse: null });
await expect(runtimeAgent.ask('x', {})).rejects.toThrow(); // or resolves.toBe('')

Consider throwing if finalResponse is nullish when an outputSchema was requested, or explicitly document that null stringifies to "null".

} finally {
activeTurns.delete(activeTurn);
}
Expand Down
6 changes: 5 additions & 1 deletion skills/rig/rig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ export type AgentOptions = {

export type AgentAskOptions = {
signal?: AbortSignal;
outputSchema?: JsonSchemaObject;
};

export interface Agent {
Expand Down Expand Up @@ -1906,7 +1907,10 @@ export function agent(spec: AgentSpec<any, any>): AgentFn<any, any> {
await runAgentAddons(runtime.addons, context, async () => {
lastResponse = await runtimeAgent.ask(
context.prompt,
context.signal ? { signal: context.signal } : undefined,
{
...(context.signal ? { signal: context.signal } : {}),
outputSchema: toJsonSchema(context.outputSchema),

@github-actions github-actions Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/codebase-design] outputSchema is now always passed (even when undefined from toJsonSchema) to every engine, not just Codex. Engines that don't use it will silently ignore it, but this couples the generic AgentAskOptions contract to a Codex-specific feature.

💡 Context

The Copilot engine (line 564 in rig.ts) discards askOptions.outputSchema today because it only reads signal. That is safe now, but any future engine that spreads askOptions directly into its SDK call could accidentally forward an unknown field. Consider documenting that engines are free to ignore outputSchema, or narrowing the propagation to engines that declare support.

},
);
context.response = lastResponse;
debugAgentResponse({ agent: normalizedSpec.name, turn, response: lastResponse });
Expand Down
23 changes: 23 additions & 0 deletions src/engines/codex.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,29 @@ it("uses Codex defaults when optional configuration is absent", async () => {
expect(mocks.startThread).toHaveBeenCalledWith({ model: "gpt-5-codex" });
});

it("forwards output schemas to Codex and stringifies structured responses", async () => {
const runtimeAgent = await codexEngine()({ model: "gpt-5-codex" });
const outputSchema = {
type: "object",
properties: {
summary: { type: "string" },
},
required: ["summary"],
additionalProperties: false,
} as const;
mocks.run.mockResolvedValueOnce({ finalResponse: { summary: "done" } });

await expect(runtimeAgent.ask("summarize", { outputSchema })).resolves.toBe(JSON.stringify({ summary: "done" }));

@github-actions github-actions Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] The test only covers the structured-object path. Add a test confirming that a plain string finalResponse is returned as-is (no double-serialisation) when outputSchema is also provided — this is the boundary between the two branches in the production code.

💡 Suggested test 
it('returns string finalResponse unchanged when outputSchema is set', async () => {
  const runtimeAgent = await codexEngine()({ model: 'gpt-5-codex' });
  mocks.run.mockResolvedValueOnce({ finalResponse: '{"summary":"done"}' });
  await expect(
    runtimeAgent.ask('summarize', { outputSchema: { type: 'object' } })
  ).resolves.toBe('{"summary":"done"}');
});

expect(mocks.run).toHaveBeenCalledWith(
"summarize",
expect.objectContaining({
signal: expect.any(AbortSignal),
outputSchema,
}),
);
});

it("rejects non-string system messages", () => {
const factory = codexEngine();

Expand Down
5 changes: 4 additions & 1 deletion src/rig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,10 @@ describe("agent invocation", () => {

await expect(greet({ text: "Hi" })).resolves.toEqual({ text: "custom" });
expect(factory).toHaveBeenCalledWith({ model: "custom-model" });
expect(ask).toHaveBeenCalledWith(expect.stringContaining("Hi"), undefined);
expect(ask).toHaveBeenCalledWith(
expect.stringContaining("Hi"),
expect.objectContaining({ outputSchema: expect.any(Object) }),
);
expect(close).toHaveBeenCalledOnce();
expect(mocks.copilotClientCtor).not.toHaveBeenCalled();
});
Expand Down