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
2 changes: 1 addition & 1 deletion API.md
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,7 @@ static ResponsesProxy.create(options?: {
| --- | --- | --- | --- |
| `authMode` | `CodexAuthMode` (`'apikey' \| 'chatgpt'`) | auto-detected | Which auth path Codex uses. Auto-detection reads `~/.codex/auth.json`; absent → `'apikey'`. |
| `upstreamBaseUrl` | `string` | derived from `authMode` | The real upstream. Defaults: `apikey` → `https://api.openai.com/v1`, `chatgpt` → `https://chatgpt.com/backend-api/codex`. |
| `eventsFile` | `string` | unset | If set, every emitted `event` is also written as a JSON line to this file (forensic mirror; `Buffer` payloads are inlined as `{ _buffer_b64 }`. Dumps written before agent-code#372's fix hold `{"type":"Buffer","data":[…]}` byte arrays instead, so a reader of older files must accept both). Written asynchronously through one ordered stream, never on the emitting call. When a line would push the total unwritten bytes (across the live and any rotated-out file) past 16 MiB, it is dropped and counted. A `{kind:'mirror-dropped', droppedEvents, droppedBytes}` line precedes the next written line, or is written at `stop()` if no line follows. Markers obey the same bounds: with a queue cap smaller than one marker (only ever a test setting), the marker is not written and the drops stay counted in the mirror's stats. Call `flushMirror()` to wait for the queue; `stop()` flushes and closes it. |
| `eventsFile` | `string` | unset | If set, every emitted `event` is also written as a JSON line to this file (forensic mirror; `Buffer` payloads are inlined as `{ _buffer_b64 }`. Dumps written before agent-code#372's fix hold `{"type":"Buffer","data":[…]}` byte arrays instead, so a reader of older files must accept both). Written asynchronously through one ordered stream, never on the emitting call. When a line would push the total unwritten bytes (across the live and any rotated-out file) past 16 MiB, it is dropped and counted. A `{kind:'mirror-dropped', droppedEvents, droppedBytes}` line precedes the next written line, or is written at `stop()` if no line follows. Markers obey the same bounds: with a queue cap smaller than one marker (only ever a test setting), the marker is not written and the drops stay counted in the mirror's stats. Also keeps the newest main-turn `responses*` request body in `latest-request-body.json` beside the file: one line, `{kind:'request-body-latest', requestId, endpoint, body_b64}` (raw on-wire bytes, zstd on current Codex), the name and kind of the Claude addon's sidecar. Requests with an output schema (`request_shape.has_output_schema`, e.g. title generation), subagent requests (`x-openai-subagent` other than `compact`), and bodiless requests never replace it. A newer body removes the old file at once. A write superseded by a newer body never lands, and a body over 16 MiB leaves no file, so the file is never an older prompt. Writes are asynchronous, encoded in 768 KiB slices, and keep at most one pending body. Call `flushMirror()` to wait for the queue and the sidecar; `stop()` flushes and closes them. |
| `eventsFileMaxBytes` | `number` | 64 MiB | When the next line would pass this size, the file is renamed to `<name>.1.jsonl` (replacing any earlier one) and a fresh file starts with `{kind:'mirror-rotated', rotatedBytes, rotations, droppedEvents, droppedBytes}`. Every file stays within the cap, markers included, so a run holds at most two files of this size. A line that cannot fit even in a fresh file is dropped and counted. A restart that appends to a file already at the cap rotates on its first event. |

`create()` starts listening before resolving. The resolved instance
Expand Down
7 changes: 7 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ While the proxy is in use:
`Authorization` header** so a leaked bundle cannot expose bearer
tokens — but it **does** record request bodies (your prompts, base64,
capped at 2 MiB) and response bytes. Treat that file as sensitive.
- With `eventsFile` set, the proxy also keeps the newest main-turn Responses
request body (up to 16 MiB) in `latest-request-body.json` next to it, so a
debug bundle has the prompt even after the events tail has moved past it
(agent-code#1336). A body between 2 MiB and 16 MiB is persisted **only**
there: the mirror omits it. A crash between writing and renaming can also
leave a `latest-request-body.json.<pid>.<n>.tmp` copy beside it. Treat all
of these as sensitive.

The proxy is **opt-in**. If you don't construct it, no interception
happens — Codex talks to the upstream directly and the package observes
Expand Down
58 changes: 58 additions & 0 deletions docs/plans/2026-09-27-latest-request-body.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Keep the newest Responses request body beside the events file (agent-code#1336)

## Evidence
- **What the bundle carries.** Agent Code's debug bundle includes the last 5 MiB of `proxy-events.jsonl` (plus `.1`).
- **Why the prompt falls out.** On Codex the response chunks are the bulk of that file. So after a long stream, or a run of `/v1/models` refreshes, the `request` event with its `body_b64` is no longer in the tail, and the bundle has no prompt.
- **Measured.** Review C of agent-code#1332 found the last inline body more than 5 MiB before EOF in 40 of 65 Codex files: 10 of 65 after the base64 fix (#53).
- **The Claude precedent.** Claude solved the same loss with `latest-request-body.json` (claude-code-headless#62, agent-code#1273). Agent Code's `readLatestRequestBody` already appends `<runDir>/latest-request-body.json` for any provider. Codex never writes one.

## Change
- **New module, `src/proxy/latestRequestBody.ts`:** `LatestRequestBodySidecar`. For each `responses*` request with a body, it replaces the sidecar with `{kind:'request-body-latest', requestId, endpoint, body_b64}`.
- **Ordered writes.** One ordered chain of async writes (temp file plus rename): no sync I/O on the main process, and an older body can never rename over a newer one.
- **Removal instead of staleness.** It removes the sidecar when the newest body is over 16 MiB (the Claude cap) or a write fails, so it never shows an older prompt as current.
- **Bodiless requests are skipped.** `/models` GETs never touch it.
- **Wiring in `ResponsesProxy`.** It is created with the mirror (same `eventsFile` opt-in) and recorded right after the `request` event. `flushMirror()` and `stop()` also wait for it.
- **Docs.** `API.md` and `SECURITY.md` document the file, including that it holds prompt text.

## Invariant, and how it differs from Claude
Claude's sidecar holds the newest body that is NOT in its log, because that log omits bodies past a budget. Codex never omits bodies up to 2 MiB; the loss is in the bundle's tail, which the proxy cannot see. So this sidecar always holds the newest Responses body, even when the log also has it. The cost is at most one body appearing twice in a bundle.

## Tests (`responsesProxy.latestBody.test.ts`, real requests through the proxy to a local upstream)
1. After two POSTs, the sidecar holds the second body, with its `requestId`, as one line.
2. A `/models` GET after a prompt leaves the prompt in place.
3. A body over 16 MiB removes an existing sidecar.

All three are red with the wiring removed.

## Then
The app bump carries this to Agent Code. Its reader's WHY comment, which describes only the Claude invariant, gets the Codex one.

## Review a (round 1)
- **Stale "latest"**, after a crash between write and rename, a failed removal, or a read while a newer write was queued. Fixed with three rules (see the module header):
- `record()` unlinks the real-name file synchronously;
- a superseded write never renames: the generation check and `renameSync` run in one synchronous turn (steering q96: an awaited async rename after the check let `record(B)` unlink the file and A's late rename restore it);
- only the newest pending body is kept.
If the directory refuses the unlink, nothing can make the file current; that stays unreported, like every mirror failure.
- **Title generation replaced the main prompt.** Codex 0.157 title turns carry an output schema (`text.format`, `codex_output_schema`). `request_shape` gains `has_output_schema`, and such requests are skipped. A body whose shape cannot be read is still kept.
- **Main-process cost.** Base64 is encoded in 768 KiB slices, one per turn after a drain, instead of 21 MiB in one call. At most one body is pending.
- **SECURITY.md accuracy.** Bodies between 2 MiB and 16 MiB are persisted only in the sidecar.
- **Fresh sessions never reach the file (P1).** This is an app-side selection bug, not a package bug: the bundle asks for `resume-<providerSessionId>` while a fresh run lives under `shell-<paneId>`. It is fixed in the app PR that bumps this package, where it is reachable and testable.
- **Tests:** compaction is recorded, the title turn is skipped, a newer record removes the file at once, a superseded write cannot resurrect an older body, and a multi-slice body round-trips.
- Mutations killed: the endpoint narrowing, the schema check, the generation check and the unlink.

## Steering q96
- **A late async rename restored an older body.** The first round checked the generation, then awaited an async `rename`. A `record(B)` in that gap unlinked the public file, and A's rename then restored A while B was still being written.
- **Fix: commit in one turn.** The commit is now `renameSync` in the same turn as the check. It is one metadata call; body bytes are still written asynchronously in slices.
- **Test: `latestRequestBody.commitFence.test.ts`.** It holds every async `fs/promises` rename at a gate and spies `renameSync`. It records B while A's commit is pending, releases A, and reads the public file before B commits: the file is absent or B, never A. It was red on `a54cfe7` (A restored).

## Review b (round 1)
- **Subagent calls replaced the main prompt.** Codex tags every non-main Responses call with `x-openai-subagent` (codex-api `requests/headers.rs`: `review`, `compact`, `thread_spawn`, `memory_consolidation`, or a label). They are now skipped, except `compact`, which carries the main conversation. There are tests for `thread_spawn` and `review`, and for a kept `compact`.
- **16 MiB cap untested.** A body of exactly 16 MiB is now kept, so lowering the cap fails a test.
- **Crash-left temp copies.** They are now listed in `SECURITY.md`.
- **P1 (the app side), no package change.** Covered by agent-code#1399, plus a bump that follows #1366. This PR is "For", not "Fixes", #1336.

## Review c
- **The endpoint filter was unpinned, and my "mutations killed" claim for it was false.** The `/models` test is bodiless, so the filter was never consulted. A new test sends bodied `/memories/trace_summarize` and `/alpha/search` POSTs after a main prompt, and the prompt must stay. Replacing the filter with `true` now fails it.
- **`has_output_schema` is a heuristic, not a title detector.** Output schemas are a per-turn option upstream. Agent Code sends none on main turns, and a structured main turn would leave the sidecar one turn behind. The comment now says so.
- **Crash-left temp files were cleaned only after a failure.** They are now also swept after a successful commit, with a test.
- **Subagent label list in the comment.** Corrected to upstream's (`collab_spawn`, `guardian`, …). The rule itself is label-agnostic.
91 changes: 91 additions & 0 deletions src/proxy/latestRequestBody.commitFence.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'

import { afterEach, expect, it, vi } from 'vitest'

// steering q96 / #70: the "never an older prompt" invariant must hold at the
// COMMIT boundary. The first fix checked the generation and THEN awaited an
// async rename. `record(B)` could run while that rename was in flight: it
// unlinked the public file, then A's rename landed and restored A as
// "latest" while B was still being written. A bundle read or a crash in that
// window labelled an older prompt current.
//
// This holds every async `rename` from fs/promises at a gate and spies the
// synchronous one, so the test can record B exactly while A's commit is
// pending, whichever API the implementation commits with.
const gate = vi.hoisted(() => ({
held: [] as Array<() => void>,
commits: 0,
}))

vi.mock('fs/promises', async importOriginal => {
const actual = await importOriginal<typeof import('fs/promises')>()
return {
...actual,
rename: async (from: string, to: string) => {
gate.commits += 1
await new Promise<void>(resolve => gate.held.push(resolve))
return actual.rename(from, to)
},
}
})

vi.mock('fs', async importOriginal => {
const actual = await importOriginal<typeof import('fs')>()
return {
...actual,
renameSync: (from: string, to: string) => {
gate.commits += 1
return actual.renameSync(from, to)
},
}
})

const { LatestRequestBodySidecar } = await import('./latestRequestBody.js')

const dirs: string[] = []
afterEach(() => {
for (const release of gate.held.splice(0)) release()
gate.commits = 0
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true })
})

async function until(predicate: () => boolean): Promise<void> {
const deadline = Date.now() + 5_000
while (!predicate()) {
if (Date.now() > deadline) throw new Error('timed out')
await new Promise(resolve => setTimeout(resolve, 1))
}
}

it('never restores an older body when a newer one is recorded during its commit', async () => {
const dir = mkdtempSync(join(tmpdir(), 'cxh-latest-body-fence-'))
dirs.push(dir)
const sidecar = new LatestRequestBodySidecar(join(dir, 'proxy-events.jsonl'))

sidecar.record('req-a', 'responses', Buffer.from('OLDER prompt A'))
// A has written its temp file and reached its commit.
await until(() => gate.commits === 1)

// B arrives now. It is large, so its own write spans many turns and cannot
// commit within the window below.
sidecar.record('req-b', 'responses', Buffer.alloc(4 * 1024 * 1024, 0x62))
for (const release of gate.held.splice(0)) release()
// Let A's commit, if it was still pending, land.
await new Promise(resolve => setTimeout(resolve, 20))

// Before B commits, the public file is absent or B: never A.
if (existsSync(sidecar.path)) {
expect(readFileSync(sidecar.path, 'utf8')).not.toContain(Buffer.from('OLDER prompt A').toString('base64'))
}

// Let every remaining write finish, releasing any rename that is held.
const releaser = setInterval(() => { for (const release of gate.held.splice(0)) release() }, 1)
try {
await sidecar.flush()
} finally {
clearInterval(releaser)
}
expect(readFileSync(sidecar.path, 'utf8')).toContain('"requestId":"req-b"')
})
84 changes: 84 additions & 0 deletions src/proxy/latestRequestBody.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'

import { afterEach, expect, it } from 'vitest'

import { LatestRequestBodySidecar } from './latestRequestBody.js'

// #70 review a: "never an older prompt" must hold at every instant a bundle
// can read the file, not only once writes settle, and a large body must not
// be encoded in one main-process turn.

const dirs: string[] = []
afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }) })
function sidecarIn(): LatestRequestBodySidecar {
const dir = mkdtempSync(join(tmpdir(), 'cxh-latest-body-unit-'))
dirs.push(dir)
return new LatestRequestBodySidecar(join(dir, 'proxy-events.jsonl'))
}
const bodyOf = (path: string) => Buffer.from((JSON.parse(readFileSync(path, 'utf8')) as { body_b64: string }).body_b64, 'base64')

it('removes the older body the moment a newer one is recorded', async () => {
const sidecar = sidecarIn()
sidecar.record('req-1', 'responses', Buffer.from('older prompt'))
await sidecar.flush()
expect(existsSync(sidecar.path)).toBe(true)

sidecar.record('req-2', 'responses', Buffer.from('newer prompt'))
// Synchronously, before any write: a reader (or a crash) now sees no body,
// never the older one.
expect(existsSync(sidecar.path)).toBe(false)
await sidecar.flush()
expect(bodyOf(sidecar.path).toString()).toBe('newer prompt')
})

it('never lets a superseded in-flight write rename over a newer body', async () => {
const sidecar = sidecarIn()
// A large body takes several turns to encode, so the next record lands
// while it is still in flight.
const large = Buffer.alloc(4 * 1024 * 1024, 0x61)
sidecar.record('req-1', 'responses', large)
sidecar.record('req-2', 'responses', Buffer.from('newest prompt'))
await sidecar.flush()

expect(bodyOf(sidecar.path).toString()).toBe('newest prompt')
// The superseded write left no temp file behind.
expect(readdirSync(join(sidecar.path, '..')).filter(name => name.endsWith('.tmp'))).toEqual([])
})

it('writes a multi-slice body that round-trips byte for byte', async () => {
const sidecar = sidecarIn()
// Not a multiple of the 768 KiB slice, and arbitrary bytes (a zstd frame is
// binary), so slice boundaries and padding are both exercised.
const body = Buffer.from(Array.from({ length: 2 * 1024 * 1024 + 7 }, (_, i) => (i * 131 + 7) % 256))
sidecar.record('req-1', 'responses', body)
await sidecar.flush()

expect(bodyOf(sidecar.path).equals(body)).toBe(true)
expect(readFileSync(sidecar.path, 'utf8').split('\n').filter(Boolean)).toHaveLength(1)
})

it('does not resurrect an in-flight older body after a newer one was too big to keep', async () => {
// The newer body is over the cap, so nothing replaces the file; only the
// superseded write's own generation check stops it renaming the older body
// back in as "latest".
const sidecar = sidecarIn()
sidecar.record('req-1', 'responses', Buffer.alloc(4 * 1024 * 1024, 0x61))
sidecar.record('req-2', 'responses', Buffer.alloc(16 * 1024 * 1024 + 1, 0x62))
await sidecar.flush()

expect(existsSync(sidecar.path)).toBe(false)
})

it('sweeps a crash-left temp file on the next successful commit', async () => {
// #70 review c: a crash between writing and renaming leaves a temp file
// holding a full prompt. It used to be cleaned only after a failed write.
const sidecar = sidecarIn()
const { writeFileSync } = await import('node:fs')
writeFileSync(`${sidecar.path}.99999.7.tmp`, 'crash-left prompt body')
sidecar.record('req-1', 'responses', Buffer.from('prompt'))
await sidecar.flush()
expect(readdirSync(join(sidecar.path, '..')).filter(name => name.endsWith('.tmp'))).toEqual([])
expect(bodyOf(sidecar.path).toString()).toBe('prompt')
})
Loading
Loading