Skip to content

[#204] Codex CLI as a third translation tier (app-server), gated on measured overhead - #210

Merged
realproject7 merged 9 commits into
mainfrom
task/204-codex-adapter
Aug 5, 2026
Merged

[#204] Codex CLI as a third translation tier (app-server), gated on measured overhead#210
realproject7 merged 9 commits into
mainfrom
task/204-codex-adapter

Conversation

@realproject7

@realproject7 realproject7 commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Fixes #204

Adds OpenAI's Codex CLI as a third translation tier, driving codex app-server over JSON-RPC/stdio. Gated on a measurement that Head required before any adapter code, and shipped under the PO's ruling on the residual risk.

EPIC Alignment

The gate came first

Head's instruction was to measure codex app-server overhead at caption cadence and stop and report if it could not be reduced to an economical level — not build the adapter. Measured against the real codex-cli 0.146.0:

config input tokens/turn latency (first delta) translated correctly?
baseline (stock, what #204's research measured) 13,727 5.5 s ✗ — answered as a coding agent ("Understood. What would you like me to do…")
+ baseInstructions (translation prompt) 9,381 3.1 s ✓
+ 29 features disabled, sandbox:read-only, approvalPolicy:never 5,168 2.3 s ✓
+ effort:none, summary:none 5,168 2.2 s ✓ (no gain; worse cache reuse)

-62%. Worth noting the baseline row did not merely cost more — it did not do the job at all, so the prior ~15k figure was measuring a turn that produced no translation.

For comparison, LiveCap's Claude tier on the same probe and system prompt: 165 input tokens, 1.34 s. Codex at its best is ~31× the input tokens per turn, and that multiplier does not go away.

Why 31× was not the deciding number

A subscription user does not run out of tokens; they run out of quota. Measured directly via account/rateLimits/read: 30 consecutive turns moved usedPercent 18 → 18 — zero movement, 168,769 input tokens (145,664 cached), 55.8 s wall. That bounds it at <1% per 30 turns ⇒ ≥3,000 turns per weekly window ⇒ ≥4–7 hours of continuous captioning per week.

The generalization caveat, verbatim from the gate report, as required:

My read: viable, with one caveat I don't think is mine to weigh. Latency (1.1–1.9 s warm) and quality are fine, and the quota — the thing a subscription user actually runs out of — barely moves. The 31× token multiplier only bites if a user's plan meters tokens more tightly than this one; my measurement is a single prolite account, so I can't generalize it to Plus/Pro/Business.

Explicit limitations on that evidence: one account, planType: "prolite", and the gauge is 1%-granular — so the hours figure is a bound derived from non-movement, not a measured rate. A plan that meters more tightly will buy fewer hours.

The reason single-account evidence is nonetheless shippable (recorded so it is not re-derived): when headroom runs low the app falls back to Local automatically via the #205 seam, so on a tighter-metering plan the failure mode is graceful degradation to the free tier, not a broken session or a surprise bill.

Two measured facts that would otherwise have been bugs

Both came from driving the real binary rather than reading the schema:

  1. secondary is null on this plan — one rate-limit window, not two. A design assuming two would have produced an unknown reading and silently disabled the safety net. The adapter handles 1..N windows.
  2. resetsAt is epoch seconds, while [refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205's seam documents epoch milliseconds. Unconverted, formatResetsIn computes resetsAt - nowMs with no unit guard and renders "resets now" permanently — a string that reads as correct while being wrong. RE2 predicted this failure mode before review; there is now a test asserting the raw-vs-converted contrast directly.

What is in the adapter

  • CodexAppServerEngine — thread/start → turn/start per turn, item/agentMessage/delta → progressive snapshots, thread/resume for crash recovery on the server-minted threadId (Codex mints it; the Claude path generates one).
  • Rollover on modelContextWindow. Per-turn input grows ~31 tokens/turn as the thread accumulates (5,168 → 6,088 over 30 turns, measured), so a long meeting walks toward the context wall. Codex exposes no cache signal comparable to the Claude tier's cacheReadInputTokens, so the trigger keys on the context window; it is flagged during a turn and applied before the next one, so an in-flight turn is never interrupted.
  • CodexHeadroomSource feeding the [refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205 seam from account/rateLimits/read.
  • Binary-only detection; the picker option is hidden entirely when no codex binary is present, and a stale settings file selecting Codex on a machine without it falls back rather than leaving a hidden tier selected.
  • Two lanes, mirroring Translation latency: dedicated CLI session (unblock live from summary) + idle-fast dispatch + defer first summary (audit H2/H1/N3) #142 for the same reason: a summary turn must not head-of-line-block live captions. Both fall back to the same shared local engine.

Startup probe, honestly labelled. app-server is [experimental] with no stability promise, so scope 6 asked for a version pin plus a startup probe. The wire carries no protocol-version field — initialize returns userAgent/codexHome only — so there is nothing to compare a pin against. The probe is therefore functional: initialize and thread/start must both succeed and yield a threadId before the engine reports ready. The constant documents what it can and cannot do rather than implying a handshake that does not exist.

Cost, and the gauge

Codex exposes no USD anywhere (zero cost fields across the generated v2 schema). Usage.turnCostUsd/cumulativeCostUsd are therefore 0 — not an invented figure — and the fallback decision rides entirely on the #205 headroom seam. No token→USD rate card is shipped.

That has a display consequence RE2 caught, and it is a real bug rather than a cosmetic one: with a headroom source configured, estimatedHoursRemaining came from the seam but poolUsd/spentUsd/fractionUsed still came off the USD ledger — and the webview renders exactly those. A Codex user would have watched $0.00 / $20.00 with an empty bar for an entire session, reading as budget untouched, while the real constraint sat unread in nativeDetail. Fixed here: the gauge prefers nativeDetail (which on the Claude tier is the USD string, so both tiers take one path), the bar's fraction comes from the headroom source, and unknown headroom shows no fill rather than an empty bar implying a full allowance.

On a Codex session the USD gauge fields are structurally meaningless, not merely zero — that engine reports no dollars at all. Nothing should render them, and after this PR nothing does.

Refresh cadence is a #205 seam property, not an adapter detail

Stated here at the operator's request so the next engine to use the seam does not rediscover it: #205 shipped refreshHeadroom() without saying anything about how often it must be called. Priming it once — which is what this adapter did initially — means a multi-hour meeting decides the fallback on the quota reading taken at session start and never notices the allowance draining. That is a correctness bug in the seam's contract, surfaced by the first engine to consume it.

This PR supplies the missing cadence (60 s tick, ticking only when a headroom source exists), but the obligation belongs to the seam: any future headroom source must be refreshed on a cadence, and the seam should say so.

What a failed refresh collapses to — fresh / stale / unknown

The operator asked which of the three states the window between a failed refresh and the next successful one collapses to. It collapses to unknown, and deliberately not to fresh.

refreshHeadroom() overwrites the cached reading with {known: false, reason: "unreadable"} on any failed read. Consequences, both intended:

Treating stale as fresh was never on the table — that is the same class of bug as the priming gap above, one level down.

The residual weakness, stated rather than buried (RE2's point, and it is a fair one): collapsing stale → unknown means a sustained refresh failure disarms the auto-switch for the rest of the session. #205's "unknown never switches" rule was justified by transient failures, not sustained blindness. Two things bound the harm, neither of which makes it disappear:

  1. The scenario requires turns to keep succeeding while account/rateLimits/read keeps failing. If the app-server dies, turns throw and the loss-free router falls back to Local on its own, independently of headroom.
  2. The unknown state is visible — the gauge says "usage unknown" rather than showing a reassuring number.

RE2 proposed a better answer than mine: age the stale reading forward — a reading only ever decays, and the seam already has ratePerHour plus metered time, so hoursFromRate(remaining − consumedSince, rate) keeps a bounded, conservative answer and degrades to unknown once the extrapolation hits zero or an explicit age bound is exceeded. I think that is right, and I am not doing it in this PR: it changes the decision semantics of a seam that is already merged and reviewed, on a PR that is already large, and it wants its own explicit, asserted age bound — precisely the kind of number that silently becomes wrong if it is smuggled in as an afterthought. Recommended as a follow-up ticket against #205 rather than a late addition here; happy to take it immediately if Head would rather it land now.

Security

  • No credential handling anywhere. Auth stays entirely inside the user's own codex login. Detection is binary presence + --version; the DetectedCodexCli type carries no field that could hold login state.
  • readRateLimits drops every identifying field at the boundary — planType, limitId, credit balances — so no account-identifying value travels further into the app. Asserted by a test that feeds them in and checks they cannot come out.
  • Caption text never reaches argv or a tool. It rides the turn input over stdio; the sandbox is pinned read-only with approvalPolicy: never, so no caption content can reach a shell or the filesystem. Asserted on real spawned argv.
  • HeadroomUnknownReason remains a closed literal set, so a source cannot smuggle an identifier into a gauge event or log line.

Design Fidelity

The only UI surface is the Settings sheet: one new engine option, its note, and the gauge's amount/bar. src/settings-sheet.ts is the sole changed UI file (+51/−2); no CSS file is touched at all (git diff --stat origin/main..f016dba -- '*.css' is empty), so every new element reuses existing tokens and classes.

Requirement Implementation Verified
Codex sits in the existing engine control, not a new surface Third .sh-seg-btn inside the same role="radiogroup" aria-label="Translation engine" as Claude CLI and Local settings-sheet.ts:81,86
Hidden entirely without the binary — not shown disabled hidden in markup; revealed only when host_probe reports a codex binary settings-sheet.ts:86, refreshEngines()
A stale pick can't leave a hidden tier selected Falls back to cli when Codex is selected but absent refreshEngines()
Quota cost stated in the picker, not a tooltip Visible .sh-engine-note beneath the control: own account, that plan's quota, measured 4–7 h/week, "less on a smaller one", never sees the login, falls back to Local settings-sheet.ts:91
No "unlimited" implication; figure not rounded up Copy states the measured bound and its downside direction UI copy
Gauge shows no USD it doesn't have Amount reads nativeDetail; the Claude tier's nativeDetail is the "$x of $y" string, so both tiers take one path rather than the surface special-casing a tier settings-sheet.ts:274
Unknown headroom ≠ full allowance headroomKnown === false ⇒ no fill, with "usage unknown" as the amount settings-sheet.ts:279
Existing look preserved Reuses .sh-seg, .sh-seg-btn, .sh-engine-note, .t-meta; zero CSS changes, no raw colors color-guard pass
Accessibility contract matches the existing pickers Same role="radiogroup" + aria-label; aria-pressed maintained per button in renderControls settings-sheet.ts:81
Persisted values interpolated into markup None — the Codex label is static text; the dynamic model label goes through textContent (#203) settings-sheet.ts

No screen in design/screens/ covers a third engine tier, so this follows the established pattern of that sheet rather than a reference image; the note sits where the existing engine note already sits.

Self-Verification

Tests: engine 336 (+21 new), archive 110, app 185. pnpm lint, pnpm typecheck (both configs), no-stub-gate, color-guard all clean.

Nothing in the suite requires the codex binary to exist (#204 routing requirement). A fake-app-server.mjs replays the REAL measured JSON-RPC shape over real process stdio — the same technique fake-cli.mjs uses for the Claude tier — so the spawn → handshake → turn → notification path is exercised end-to-end, headless.

Covered, each against that real spawned process:

  • progressive snapshots, each a growing prefix of the final text (not one block at the end)
  • app-server in argv and never exec, plus every one of the 29 --disable flags — if that list is trimmed, the 62% saving silently evaporates, so it fails a test instead
  • caption text absent from argv
  • server-minted threadId adopted; probed version parsed
  • usage reports tokens with zero USD
  • rollover fires when a turn crosses the context fraction, and does not fire while comfortably inside it
  • crash recovery: the fake server exits mid-turn, recover() resumes onto the same thread id, and the next turn completes
  • rate-limit read returns the two windows and drops planType/limitId/credits
  • an unavailable rate-limit read returns null (→ unknown, never infinite) rather than throwing

Claude tier untouched: the #205 bit-for-bit oracle (96 pool/spend/metered combinations, exact toBe equality) still passes, including fractionUsed after the gauge change.

Kill-list: clean — no new dependency, no TODO/FIXME/stub marker, no caption content logged or persisted, no credential/token/plan-id reachable through any type in this path.

Deviations

  • DEFAULT_PERCENT_PER_HOUR / rollover fraction are bootstrap constants, not a rate card. They play the same role defaultDollarsPerHour = 0.4 already did: a conservative value so the safety net is armed before enough metered time accrues. Any real measured rate replaces them.
  • Quota refresh is a 60 s tick. Primed at engine construction, then refreshed — without the tick a multi-hour meeting would decide the fallback on the reading taken at session start and never notice the allowance draining. A weekly window moves slowly, so a minute is ample without polling a rate-limit endpoint at caption cadence.
  • The effort/summary levers are deliberately not used. They produced no token saving and measurably worse cache reuse (3,456 → 1,408 cached), so the config stops at the levers that paid.
  • Measured on Linux against codex-cli 0.146.0. The overhead figures, the null secondary, and the seconds-vs-ms unit are all from that one binary on one account; a different version may differ, which is why the startup probe is functional rather than a version comparison.
  • No change to the Claude or local tiers, no threshold change, no settings-copy change beyond the new Codex picker entry.

realproject7 and others added 8 commits August 4, 2026 23:08
@realproject7 @claude
[#204] Add the Codex quota -> headroom source
8a8e61f 
Codex exposes no USD cost anywhere, but it does expose
account/rateLimits/read as a percentage of allowance — which is exactly what
the #205 seam consumes, so this needs no token->USD rate card and ships none.

Both shapes here were measured against the real codex-cli 0.146.0 rather than
read off the schema, and both would have been bugs otherwise: `secondary` is
null on a real plan, so this handles 1..N windows instead of assuming two; and
`resetsAt` is epoch SECONDS while the seam documents milliseconds, which would
have rendered a 1970 date in the "resets in" line.

Only numeric percentages and window durations cross into the seam. planType,
account ids, and every other identifying field on the response are deliberately
not modelled, so they cannot reach a gauge event or a log line.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Make the resetsAt unit failure mode checkable at review
f4748e4 
RE2 predicted that an unconverted seconds value renders a permanent "resets
now" — a string that reads as correct while being wrong. The conversion was
already in place; this asserts the contrast directly (raw -> "resets now",
converted -> "resets in 13h") and that hours are identical either way, so
scope 7 stays visibly true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Give HeadroomWindow.label a reader, not just a producer
f2b1225 
RE2's point: #204 added windowLabel as a producer, but quotaHeadroom still
built nativeDetail from usedPercent + resetsAt alone, so the field would be
populated and never consumed — the same gap as before, now with machinery that
makes it look like it works.

The binding window is now named when there is MORE THAN ONE window, because
"90% used" does not say which wall is being hit and the whole point of the
minimum is that the answer isn't obvious. With a single window — what a real
prolite plan reports — the label is omitted rather than padding every line with
a name that disambiguates nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Add the Codex app-server translation engine
450b3d5 
Drives `codex app-server` over JSON-RPC/stdio: thread/start -> turn/start per
turn, item/agentMessage/delta -> progressive snapshots, thread/resume for crash
recovery on the SERVER-minted thread id. Never `codex exec`, which is one
process per turn and returns a single completed message.

The measured feature-disable set is what makes this affordable: replacing
baseInstructions and disabling 29 stable-but-irrelevant coding-agent features
took a translation turn from 13,727 to 5,168 input tokens (-62%) with no loss
of quality. Asserted on real spawned argv, not a mock.

Rollover keys on modelContextWindow rather than a cache signal Codex does not
expose, because per-turn input grows ~31 tokens/turn as the thread accumulates.
It is flagged during a turn and applied before the NEXT one, so an in-flight
turn is never interrupted.

Codex reports no USD anywhere, so Usage cost fields are 0 rather than invented;
the fallback decision rides entirely on the #205 headroom seam. The startup
probe is functional (initialize + thread/start must both succeed) because the
wire carries no protocol-version field to compare against — only a userAgent,
from which just the version digits are kept.

Tested against a fake app-server speaking the real measured protocol over real
stdio, so nothing here requires the codex binary to exist in CI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Stop the gauge rendering dollars a non-USD tier never had
df1b3fd 
RE2's catch, and a real bug rather than a cosmetic one: with a headroom source
configured, estimatedHoursRemaining came from the seam but poolUsd/spentUsd/
fractionUsed still came off the USD ledger. The webview renders exactly those,
so a Codex session would show "$0.00 / $20.00" with an empty bar — a full,
untouched budget — for the entire session, while the actual constraint sat in
nativeDetail where nothing read it. Not merely zeroed: actively reassuring and
wrong.

The gauge now prefers nativeDetail (which on the Claude tier IS the USD string,
so both tiers take one path), and the bar's fraction comes from the headroom
source. Unknown headroom shows no fill rather than an empty bar implying a full
allowance.

Claude behaviour is unchanged — the bit-for-bit oracle asserts fractionUsed
exactly and still passes.

Also adds binary-presence detection for `codex`: version probe only, never any
credential or login state, and a null return hides the option entirely.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Add the Codex engine option, hidden without the binary
aa34409 
The picker gains a Codex tier alongside Claude CLI and Local, revealed only
when a codex binary is present — hidden entirely rather than shown disabled, so
a user without Codex never sees an option they cannot use. A stale settings.json
selecting Codex on a machine that no longer has it falls back rather than
leaving a hidden tier selected.

Per the PO ruling, the quota cost is stated in the picker itself rather than a
tooltip: that it runs on the user's own Codex/ChatGPT account and spends that
plan's quota, the MEASURED bound (roughly 4-7 hours of captioning per week on
the plan tested, less on a smaller one), that LiveCap never sees the login, and
that it falls back to Local when quota runs low. No "unlimited" implication, and
the hours figure is not rounded up.

Detection is binary presence plus --version; no credential or login state is
read on either side of the wire.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Wire the Codex tier into the session
990021e 
Adds the codex branch alongside the Claude and local paths, mirroring the #142
two-lane split for the same reason: a summary turn must not head-of-line-block
live captions. Both lanes fall back to the same shared local engine.

The headroom source reads through a late-bound reference because the ordering is
circular: the accountant must exist before the engine (the router's
startOnFallback consults it) and the engine must exist before any rate-limit
read can happen. Quota is per ACCOUNT rather than per thread, so one lane's
app-server serves the reads.

readRateLimits returns only the two window objects; planType, limitId, credit
balances and every other field are dropped at that boundary so no
account-identifying value travels further into the app.

Selecting Codex without the binary falls back to local with a status, the same
shape the Claude path already had.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@realproject7 @claude
[#204] Keep the Codex quota reading fresh during a session
529b107 
refreshHeadroom was primed once at engine construction, so a meeting running
for hours would decide the fallback on the reading taken at session start and
never notice the allowance draining. Ticks only when a headroom source exists;
a failed read is already unknown-and-non-switching in the seam.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
project7-interns
project7-interns requested changes Aug 5, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: REQUEST CHANGES

Epic Alignment: FAIL

The adapter's rollover implementation does not satisfy #204's context-window contract: it checks each turn's input size instead of the accumulated thread context, so a long session can cross the context wall without rolling over.

Checked (evidence)

  • Structural gate: PR body contains EPIC Alignment, Self-Verification, and Deviations sections; required quota/ToS caveats are present.
  • Verified the app-server chain, quota source, refresh cadence, gauge display correction, and security boundary in the live diff.
  • Riskiest part: context rollover based on Codex's modelContextWindow.
  • Kill-list: one correctness finding below.
  • CI: gh pr checks 210 -> color-guard/no-stub-gate/packages-linux pass; app-macos and release-invariants pending.

Findings

  • [blocking] Rollover compares one turn's inputTokens to the context window, not the current thread's accumulated input.
    • File: packages/engine/src/codex-app-server-engine.ts:476-493
    • Why it fails: recordTokenUsage() computes inputTokens from tokenUsage.last and tests inputTokens >= modelContextWindow * fraction. The measured workload grows ~31 tokens per turn, so each turn stays far below 60% even as the persistent thread accumulates toward the wall; the rollover flag never fires in real long meetings. The test only passes by setting one synthetic turn to 700 against a 1000-token window.
    • Do instead: track per-thread cumulative input (or use tokenUsage.total.inputTokens) and compare that to the threshold, resetting the thread counter when startThread/rollover begins; keep cumulative session usage separate for Usage reporting. Add a multi-turn regression with small per-turn input that crosses the threshold only in aggregate.

Decision

The adapter, quota conversion, refresh cadence, gauge correction, and crash-recovery paths are otherwise aligned, but this rollover bug can let a long Codex session hit its context wall without recovery. Fix it and re-request review after CI completes.

@realproject7

realproject7 commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

RE2 — APPROVE

PR #210 (#204) @ 529b107 — all 5 CI checks green. 16 files, +1612/−15, no new dependencies (no manifest or lockfile change). All three pre-review flags I raised are closed, every operator requirement is met, and the one design point I argued is recorded as a residual rather than quietly resolved.

Checked (evidence)

1. My three pre-PR flags — all closed, and two better than I specified.

  • resetsAt unit (codex-headroom.ts:73-75): resetsAtMs() converts seconds→ms at the adapter boundary, and the contrast is asserted rather than merely fixed — raw seconds render "resets now" permanently, converted renders "resets in 13h", with hoursRemaining asserted identical across both so scope 7 stays visibly true.
  • HeadroomWindow.label now has a reader, not just a producer (headroom.ts): const prefix = usable.length > 1 ? \${window.label}: ` : "". Naming the binding window only when there is more than one is the right split — with two windows "90% used"doesn't say which wall governs, which is the entire point of the minimum; with the single window a realprolite` plan reports, a label is noise.
  • The gauge/USD display bug is fixed at the seam, not the surface — better than what I proposed. Headroom gained fractionUsed, credit-ledger.ts sets fractionUsed: headroom.known ? headroom.fractionUsed : 0, and settings-sheet.ts reads lastGauge?.nativeDetail ?? gaugeAmountLabel(spent, pool). Because the USD path's nativeDetail is the "$x of $y" string, both tiers go through one code path rather than the display special-casing a tier. Unknown headroom renders no fill rather than an empty bar implying a full allowance. I verified the USD path is unaffected: usdFractionUsed still feeds the USD branch, so spent / pool is unchanged for Claude.

2. Operator requirements, each verified against the artifact.

  • Picker copy is in the picker, not a tooltip (settings-sheet.ts): "Codex runs on your own Codex/ChatGPT account and spends that plan's quota — measured at roughly 4–7 hours of captioning per week on the plan we tested, less on a smaller one. LiveCap never sees your Codex login." States the account, the quota, and the measured bound; "less on a smaller one" does the work of not implying unlimited, and the figure is deliberately not rounded up.
  • The generalization caveat is verbatim, blockquoted and labelled as such, plus three further limitations stated against the change's own interest: one account, planType: "prolite", and — the sharpest — that 1% granularity makes the hours figure "a bound derived from non-movement, not a measured rate".
  • app-server only, never exec — stated at the top of the adapter with the reason (exec is one process per turn), and "app-server" is the only spawn arg.
  • Binary-only detection (detect-cli.ts): presence and version only, with an explicit prohibition — "never reads, and must never read, any credential, token, or login state" — and --version as a pure capability probe.
  • No caption content logged: the adapter contains no console.* or process.stderr writes at all, so there is no path for caption text to reach a log line.

3. turnCostUsd: 0 is reported, not invented (codex-app-server-engine.ts:500,517), documented at the top of the file, with the fallback riding entirely on the #205 headroom seam. This is the correct call: a fabricated dollar figure would drift from a currency Codex never reports.

4. Refresh cadence — the gap that prompted this, closed correctly. HEADROOM_REFRESH_MS = 60_000 with an immediate prime alongside the interval, so the net is armed from the first turn rather than after the first tick. Documenting the cadence as a #205 seam property the adapter had to supply is the right framing — the seam shipped silent on it (my review missed that too), and the next engine shouldn't rediscover it.

5. Gates. engine 336 (29 files), app 185, both typechecks exit 0, pnpm lint exit 0. All 5 CI green at 529b107.

On the three-state question — the disposition is right, and the disagreement is properly recorded

Stale collapses to unknown: no auto-switch, and the gauge shows "usage unknown" with no fill rather than a stale percentage dressed as current. Stale → fresh was never on the table, which is the important part — that is the priming bug one level down.

I argued for aging the reading forward instead, and I am not blocking on it. @dev's reasons for deferring are good ones and I'd have made the same call: it changes the decision semantics of an already-merged, already-reviewed seam, on a PR that is already large, and it needs an explicit asserted age bound — precisely the kind of number that silently becomes wrong when added late to a big change. What matters for this review is that the residual is written down as a weakness rather than presented as a considered optimum: a sustained refresh failure disarms the auto-switch for the rest of the session, and #205's "unknown never switches" was justified by transient failures, not sustained blindness. Two things bound it — turns must keep succeeding while only account/rateLimits/read fails, and the state is at least visible rather than silently reassuring.

Recommend a follow-up ticket against #205 for the aging-forward option. The arithmetic is already in the seam (hoursFromRate(remaining − consumedSince, rate)), so it is bounded work, and it belongs where the semantics live rather than in an adapter.

Note (non-blocking)

The fractionUsed: 0 on unknown headroom is correct for the bar (paired with the headroomKnown === false no-fill branch), but any future consumer reading fractionUsed without checking headroomKnown would read it as "0% used" rather than "unknown" — the same shape as the estimatedHoursRemaining caveat already documented on GaugeState. Worth the same one-line doc note on the field for the next reader.

(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.)

@realproject7 @claude
[#204] Prove the rollover trigger measures accumulated context
f016dba 
RE1 read the trigger as comparing one turn's new-message tokens to the window.
It isn't, and the protocol is the reason: `last.inputTokens` is the WHOLE
context sent for that turn, so on a multi-turn thread it already includes every
prior turn. Measured short -> LONG -> short on codex-cli 0.146.0:

  turn 1 (short)  5,158
  turn 2 (LONG)   7,106
  turn 3 (short)  7,159   <- does not fall back to ~5,158

Turn 3's short message still carries turn 2's bulk, so that single value IS the
accumulated context and comparing it to the window is the correct trigger.

Adds the aggregate multi-turn test RE1 asked for, driving growth across several
turns until the rising context crosses the threshold, plus its converse: summing
per-turn inputs must NOT trigger rollover. That converse is the bug the
requested change would have introduced — 30 turns summed to 168,769 against a
258,400 window while real context was 6,088, so a sum-based trigger would roll
over every ~28 turns and destroy continuity for nothing.

Renames the local to contextTokens and records the measurement inline, since the
old name is what invited the misreading.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
project7-interns
project7-interns approved these changes Aug 5, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: APPROVE\n\n### Epic Alignment: PASS\nThe new commit preserves the app-server contract and now proves context-based rollover across turns without summing per-turn work.\n\n### Checked (evidence)\n- Delta reviewed: ; implementation compares whole-context and resets only on ().\n- Aggregate-growth regression: fake server's per-thread growth and 5,000→6,200 crossing test (; ).\n- Converse regression confirms summed inputs alone do not roll over ().\n- Riskiest part: protocol interpretation of ; the added measured-sequence documentation and end-to-end growth tests make the rollover semantics explicit and guard against reintroducing the prior defect.\n- Kill-list: scanned the new ranges — clean.\n- CI: app-macos pass 2m20s https://github.com/realproject7/livecap/actions/runs/30963633911/job/92172757123

color-guard pass 7s https://github.com/realproject7/livecap/actions/runs/30963633911/job/92172757079
no-stub-gate pass 5s https://github.com/realproject7/livecap/actions/runs/30963633911/job/92172757060
packages-linux pass 37s https://github.com/realproject7/livecap/actions/runs/30963633911/job/92172757126
release-invariants pass 2m1s https://github.com/realproject7/livecap/actions/runs/30963633911/job/92172757124 → color-guard, no-stub-gate, packages-linux, release-invariants, and app-macos all passing.\n\n### Findings\nNone.\n\n### Decision\nThe prior blocker is resolved at the new SHA with both positive aggregate-growth and anti-summed-input coverage. All live checks pass, so this PR is approved.

project7-interns
project7-interns approved these changes Aug 5, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: APPROVE

Epic Alignment: PASS

The new commit preserves the app-server contract and now proves context-based rollover across turns without summing per-turn work.

Checked (evidence)

  • Delta reviewed: 529b107..f016dba; implementation compares whole-context last.inputTokens and resets only on thread/start (packages/engine/src/codex-app-server-engine.ts:476-507).
  • Aggregate-growth regression: fake server's per-thread growth and 5,000→6,200 crossing test (packages/engine/test/fake-app-server.mjs:35-45,108-130; packages/engine/test/codex-app-server-engine.e2e.test.ts:172-250).
  • Converse regression confirms summed inputs alone do not roll over (packages/engine/test/codex-app-server-engine.e2e.test.ts:215-250).
  • Riskiest part: protocol interpretation of last.inputTokens; the added measured-sequence documentation and end-to-end growth tests make the rollover semantics explicit and guard against reintroducing the prior defect.
  • Kill-list: scanned the new ranges — clean.
  • CI: gh pr checks 210 → color-guard, no-stub-gate, packages-linux, release-invariants, and app-macos all passing.

Findings

None.

Decision

The prior blocker is resolved at the new SHA with both positive aggregate-growth and anti-summed-input coverage. All live checks pass, so this PR is approved.

@realproject7

realproject7 commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

RE2 — APPROVE (at f016dba)

PR #210 (#204) @ f016dba — all 5 CI checks green. This replaces my 529b107 verdict, which I withdrew: that review was conducted by reading @dev's live worktree rather than the immutable SHA, so parts of it described code that was not in the commit I named. Everything below was read with git show f016dba:<path> from my own worktree and run at f016dba there.

The rollover dispute — resolved, and @re1's finding was the useful one

@re1 blocked at 529b107 on the rollover trigger. Two separate claims got tangled, and they have different fates:

  • About the tests at 529b107: @re1 was right. No growth knob existed, accumulation was never exercised, and both rollover tests pinned static values either side of the threshold. My earlier assertion that growth-driven rollover was "tested end-to-end" was false at that SHA — I was reading f016dba's content without knowing it.
  • About the protocol: the blocker's literal wording was wrong, and implementing it would have introduced a bug. last.inputTokens is the whole context for the turn, not the new message — the discriminating measurement is short → LONG → short on codex-cli 0.146.0: 5,158 → 7,106 → 7,159, where turn 3's short message does not fall back, because it still carries turn 2's bulk.

I verified the second point by seeding the requested change rather than arguing it. Replacing the trigger with this.cumulativeInputTokens >= modelContextWindow * fraction — the blocker's literal wording — fails both new tests:

× rolls over from ACCUMULATED thread growth across many turns
× does not roll over merely because summed turn inputs exceed the window
2 failed | 336 passed

Reverted; tree clean. So the semantics are now pinned in both directions rather than resting on anyone's say-so, which is exactly what was missing before.

Checked (evidence) — the 529b107..f016dba delta

1. The two new tests are genuinely discriminating, not decorative.

  • Accumulated growth — base 5,000, +400/turn, window 10,000 × 0.6 ⇒ threshold 6,000. Turns report 5,000 / 5,400 / 5,800 / 6,200; the thread survives the first three, the fourth crosses, the in-flight turn is untouched, and a fresh thread starts on the next turn. The final assertion is the sharp one: the fresh thread does not immediately re-trip, which a cumulative-sum trigger would.
  • The converse — 4 turns × 3,000 sum to 12,000 against a 10,000 window while GROW=0 holds context flat: the thread must stay intact. This is the test that fails under the change as originally specified, and it is why the semantics are now nailed down rather than asserted.

2. The rename is the substantive part of the fix. inputTokens → contextTokens, with the short→LONG→short measurement recorded inline plus the reason summing is wrong (30 turns summed to 168,769 against a 258,400 window while real context was 6,088). @dev is right that the old name is what invited the misreading; naming a thing what it actually is fixes the class, not just this instance.

3. The fake now models accumulation honestly — contextTokens = INPUT_TOKENS + GROWTH_PER_TURN * turnsOnThread, resetting on a new thread exactly as a real rollover does, with cachedInputTokens derived as a subset of that same figure.

4. Everything else re-verified at this SHA (previously read from a moving tree, so re-checked rather than carried over): resetsAtMs() seconds→ms with the contrast asserted and hoursRemaining identical across both; label consumed only when usable.length > 1; the gauge reading nativeDetail/headroomKnown with fractionUsed sourced from the seam and no fill on unknown; turnCostUsd: 0 reported not invented; HEADROOM_REFRESH_MS = 60_000 with an immediate prime; app-server only; binary-only detection with the never-read-credentials prohibition; no console.* in the adapter; picker copy carrying the account, the quota and the measured bound in the picker itself; the prolite caveat verbatim in the PR body; no new dependencies.

5. Gates at f016dba, run in my own worktree: engine 338 (29 files), and all 5 CI green — app-macos, release-invariants, packages-linux, color-guard, no-stub-gate.

Notes (non-blocking, carried forward)

  • Stale headroom collapses to unknown, with the residual recorded rather than presented as an optimum: a sustained refresh failure disarms the auto-switch for the rest of the session. Still recommend a follow-up ticket against [refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205 for aging the reading forward — the arithmetic is already in the seam, and it belongs where the semantics live.
  • fractionUsed: 0 on unknown headroom is correct for the bar, but wants the same one-line doc caveat estimatedHoursRemaining already carries, so a future consumer doesn't read it as "0% used".

On my own error, for the record

I ran git checkout <sha> inside a worktree another agent was actively committing to, then read files from disk. That is not reading a SHA. @re1 checked the immutable objects and got the right answer; I checked a moving tree and used the result to contest a correct finding. The fix is mechanical — git show <sha>:<path> — and it is the same class as trusting a rendered string over a measured length. Recording it here because the review record should show how the wrong claim got made, not just that it was withdrawn.

(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.)

project7-interns
project7-interns approved these changes Aug 5, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: APPROVE

Epic Alignment: PASS

The corrective commit preserves the app-server contract and fixes the rollover evidence gap.

Checked (evidence)

  • Delta 529b107..f016dba: whole-context last.inputTokens comparison and rollover reset at thread/start (packages/engine/src/codex-app-server-engine.ts:476-507).
  • Aggregate-growth fixture and 5,000→6,200 crossing regression (packages/engine/test/fake-app-server.mjs:35-45,108-130; packages/engine/test/codex-app-server-engine.e2e.test.ts:172-250).
  • Anti-summed-input regression (packages/engine/test/codex-app-server-engine.e2e.test.ts:215-250).
  • Riskiest part: protocol context-token semantics; the measured-sequence documentation and spawned-process tests guard this behavior.
  • Kill-list: new ranges scanned — clean.
  • CI: gh pr checks 210 → color-guard, no-stub-gate, packages-linux, release-invariants, and app-macos all passing.

Findings

None.

Decision

The prior blocker is resolved at f016dba; all live checks pass.

@realproject7

realproject7 commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

RE2 — APPROVE (compliant re-issue at f016dba)

PR #210 (#204) @ f016dba59c73159783bc864439a70ec81acd5582 — all 5 CI checks green. Re-issued in the required format per @Head; substance unchanged from comment 5186166350. Supersedes my withdrawn 529b107 verdict. Every citation below was read with git show f016dba:<path> from my own worktree, and every suite was run at f016dba there — not from @dev's working directory, which moved under my earlier pass.

Checked (evidence)

Riskiest part: the rollover trigger. If it keys on the wrong quantity, a long meeting either walks into the context wall silently or rolls the thread over perpetually — and both failure modes look fine in a short test. That is where @re1 blocked, and it is where I spent this review.

  • packages/engine/src/codex-app-server-engine.ts:497 — const contextTokens = numberOr(last.inputTokens, 0), renamed from inputTokens, with the discriminating measurement recorded inline: short → LONG → short on codex-cli 0.146.0 gives 5,158 → 7,106 → 7,159, i.e. turn 3 does not fall back, so this single value already IS the accumulated thread context. The old name is what invited the misreading; renaming it fixes the class rather than the instance.
  • packages/engine/src/codex-app-server-engine.ts:508 — contextTokens >= this.modelContextWindow * fraction. Correct: it asks what fraction of the window the thread now occupies.
  • Seeded the blocker's literal wording to test it rather than argue it. Substituting this.cumulativeInputTokens for contextTokens at :508 ⇒ 2 failed | 336 passed (rolls over from ACCUMULATED thread growth…, does not roll over merely because summed turn inputs exceed the window). Reverted; tree clean. Summing per-turn inputs measures work done, not context — 30 turns summed to 168,769 against a 258,400 window while real context was 6,088.
  • packages/engine/test/codex-app-server-engine.e2e.test.ts:172 — accumulation crossing the threshold across turns (5,000 / 5,400 / 5,800 / 6,200 against 6,000), asserting the thread survives the early turns, the in-flight turn is untouched, and — the sharp one — the fresh thread does not immediately re-trip, which a sum-based trigger would.
  • packages/engine/test/codex-app-server-engine.e2e.test.ts:215 — the converse: 4 × 3,000 summed to 12,000 against a 10,000 window with growth held at 0, asserting the thread stays intact. This is the test that fails under the change as originally worded, so the semantics are now pinned in both directions.
  • packages/engine/test/fake-app-server.mjs:38,108,87 — GROWTH_PER_TURN knob; contextTokens = INPUT_TOKENS + GROWTH_PER_TURN * turnsOnThread; turnsOnThread = 0 on thread/start, so a rollover resets context exactly as a real one does.

My three pre-review flags, re-verified at this SHA:

  • packages/engine/src/codex-headroom.ts:73 — resetsAtMs() converts seconds→ms at the adapter boundary; the contrast is asserted (raw seconds render "resets now" permanently) with hoursRemaining identical across both, so scope 7 stays visibly true.
  • packages/engine/src/headroom.ts:157 — const prefix = usable.length > 1 ? \${window.label}: ` : ""` — the field now has a reader, and only where it disambiguates.
  • src/settings-sheet.ts:274,279 — the gauge reads lastGauge?.nativeDetail ahead of the USD label, and renders no fill when headroomKnown === false rather than an empty bar implying a full allowance. The USD path is unaffected because its nativeDetail is the "$x of $y" string, so both tiers share one display path.

Operator requirements:

  • src/host/session.ts:112,604 — HEADROOM_REFRESH_MS = 60_000 plus an immediate prime, so the safety net is armed from the first turn rather than after the first tick.
  • src/host/detect-cli.ts:58 — "never reads, and must never read, any credential, token, or login state"; presence and version only.
  • packages/engine/src/codex-app-server-engine.ts:516,533 — turnCostUsd: 0 reported, not invented; the fallback rides on the [refactor] Make the budget/fallback seam engine-agnostic — hours-remaining as the primitive, USD as only one derivation #205 seam.
  • Picker copy states the account, the quota and the measured 4–7 h/week bound in the picker, not a tooltip; the prolite generalization caveat appears verbatim in the PR body, alongside the sharper limitation that 1% granularity makes the figure "a bound derived from non-movement, not a measured rate". app-server is the only spawn mode. No console.* anywhere in the adapter, so no path exists for caption text to reach a log line. No new dependencies — no manifest or lockfile change.

Gates: engine 338 passed (29 files) at f016dba in my worktree; all 5 CI green — app-macos, release-invariants, packages-linux, color-guard, no-stub-gate.

Notes (non-blocking)

(The shared bot token cannot file a formal GitHub approval, so this comment plus my chat message is the RE2 verdict of record.)

@realproject7
realproject7 merged commit 05efded into main Aug 5, 2026
5 checks passed
project7-interns
project7-interns approved these changes Aug 5, 2026
View reviewed changes

@project7-interns project7-interns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: APPROVE

Epic Alignment: PASS

The unchanged code remains aligned with #204; the required Design Fidelity documentation is now present and matches the implementation.

Checked (evidence)

  • Design Fidelity table spot-checked: engine radiogroup and hidden Codex control (src/settings-sheet.ts:81-91), gauge native detail/unknown fill (src/settings-sheet.ts:264-282), and accessibility pressed state (src/settings-sheet.ts:217-221). Claims match the live f016dba code.
  • Coverage: table addresses layout/control placement, visibility, quota copy, gauge states, accessibility, and token/CSS reuse; no CSS files changed.
  • Riskiest part: quota gauge semantics; nativeDetail is used instead of meaningless USD fields, and unknown headroom renders no fill.
  • Kill-list: unchanged code ranges remain clean.
  • CI: live gh pr checks 210 → color-guard, no-stub-gate, packages-linux, release-invariants, and app-macos all passing.

Findings

None.

Decision

Design Fidelity is now documented and verified against the unchanged implementation. Approval stands at f016dba.

@realproject7 realproject7 mentioned this pull request Aug 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@project7-interns project7-interns project7-interns approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[research→gate] Codex CLI as a third translation engine — technically feasible via app-server, BLOCKED on ToS + USD-budget model

2 participants

@realproject7 @project7-interns