Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BunsDev
left a comment
BunsDev
left a comment
There was a problem hiding this comment.
Review of 7ca83dbbeabeb24baf77ded7a36af7fca3f9cdf7: two changes are needed before protected landing.
-
P1 — Local execution is reachable before native destination/policy qualification.
index.ts:216constructsLocalExecutionunconditionally, andrunner.ts:1154–1174admits a local target through the existingthread:updatepath. The private-home/provider setup inlocal-models/execution.ts:148–167has no admission gate for redirect behavior or managed-policy precedence. The receipt explicitly leaves both unqualified, but #379 AC6 requires the validated destination or safe refusal and no policy bypass. Main-only/no new UI does not make this path inactive. Keep product admission unavailable until that boundary is qualified, or implement and verify the required refusal boundary; confirming the provider name does not verify the effective HTTP destination. -
P2 — Local requests still force an unqualified reasoning-summary option.
backends/codex.ts:80always starts the child withmodel_reasoning_summary="detailed"; the config at line 308 spreadsTHREAD_CONFIG, which sets the same option at line 780. Removing effort/fast options inexecution.tsleaves both settings intact. Omit this option for local configuration unless runtime/model support has been qualified, and cover both child arguments and thread configuration in the regression.
Separate dependency reconciliation: #403 did not include the later credential-copy and stale-discovery fixes preserved in 5ee2ecdbaac3226c5d6f158dbdf15b14f91eb9f5. Those are inherited main defects, not introduced by this PR. I am preparing the bounded five-file source/test repair on a fresh current-main branch, preserving the previous validation and hosted performance failure. The closed #401 is not proof those fixes landed.
This is source review, independently checked against the exact head; no live inference or CI retry was performed. The scripted native shell receipt remains distinct from AC1's real local-model acceptance.
Code coverage overviewLanguages: TypeScript TypeScript / code-coverage-agent
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
A thread with a localTarget runs on an isolated codex app-server: a private empty home, an environment allowlist, the connection token in one variable the tool shell cannot see, and the provider and model pinned on every open with fallback off and Codex's confirmation required before any content is sent. A resume handle the child did not open is refused, as is a stale sign-in retry. Local turns bypass cloud accounts, Auto, Jev, titles, follow-ups and the credit fallback; Plan, images and the note and knowledge tools are refused. One turn at a time per loopback port; force-stop ends only the thread's child. Clearing the target returns the thread to the default Codex model with a fresh handle. The opt-in native test runs the installed CLI against a scripted loopback Responses server (MODEX_LM3_WIRE) or a person's own server (MODEX_LM3_LIVE). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…Codex (#379) Addresses Val's 2026-10-26 review of #417. Main no longer constructs LocalExecution, so a built app refuses every local selection and persisted target until LM-5 qualifies Codex's redirect handling and managed-policy precedence (AC6); the module and runner seam remain for fixtures and the opt-in native checks. CodexBackend omits model_reasoning_summary from a local child's arguments and thread configuration (AC5); cloud instances are unchanged. Regressions: local-execution-wiring (main source does not import, construct or inject the module), runner-local-models (selection refused and nothing persisted without the seam) and local-model-execution (no reasoning-summary argument, empty thread config). Evidence and the re-run wire receipt are in the LM-3 review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Both review findings on
RED: the wiring test failed on the import and the execution test on the argument. GREEN: focused suites 55/55; |
|
Follow-up: #419 ( |
|
Hosted checks on |
Preserves the exact reviewed source tree from PR #417 while replacing its unsigned ancestry with a signed delivery commit.
#380) Before a local child opens or resumes a thread, CodexBackend reads the child's own config/read (effective configuration, the layer that set each key, every layer's contents and hash) and configRequirements/read, and admitLocalConfiguration (engine/local-models/run-policy.ts) admits it only when every governed key comes from Modex's session flags, no project layer is loaded, the private home is empty, no managed layer touches a governed key, no managed requirements apply, and the effective destination, wire, credential reference, context and tools are exactly what Modex passed. Otherwise the turn fails naming the layer and key; nothing is rewritten or sent. The admitted provenance is kept with the child. A model/rerouted notification on a local turn interrupts it and reports it failed. Real-CLI evidence (codex-cli 0.164.0): config/read reports origins and layers as the policy expects; a requirements.toml in the private home is not read; a 307 from the configured loopback server is followed with the full body and never with the token, a 302 as a bodyless GET. The opt-in native test records both. This is the refusal boundary #417's review asked for; wiring local execution into the product stays a separate decision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* LM-5 slice 1: admit a local Codex child on its effective configuration (#380) Before a local child opens or resumes a thread, CodexBackend reads the child's own config/read (effective configuration, the layer that set each key, every layer's contents and hash) and configRequirements/read, and admitLocalConfiguration (engine/local-models/run-policy.ts) admits it only when every governed key comes from Modex's session flags, no project layer is loaded, the private home is empty, no managed layer touches a governed key, no managed requirements apply, and the effective destination, wire, credential reference, context and tools are exactly what Modex passed. Otherwise the turn fails naming the layer and key; nothing is rewritten or sent. The admitted provenance is kept with the child. A model/rerouted notification on a local turn interrupts it and reports it failed. Real-CLI evidence (codex-cli 0.164.0): config/read reports origins and layers as the policy expects; a requirements.toml in the private home is not read; a 307 from the configured loopback server is followed with the full body and never with the token, a 302 as a bodyless GET. The opt-in native test records both. This is the refusal boundary #417's review asked for; wiring local execution into the product stays a separate decision. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix: retain rerouted local turns until native termination * docs: record integrated LM5 delivery validation --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # docs/status.md Co-authored-by: BunsDev <68980965+BunsDev@users.noreply.github.com>
Implements LM-3 (#379) under #376, consuming LM-1 (#405) and LM-2 (#403). Main-only: no UI, no new IPC channel, no inference client, and not admitted in the product (see the review repair). Refs #379, #376.
Review repair (2026-10-27,
9159eda, rebased ontomainat6c9000f). Both findings from the 2026-10-26 source review are addressed. (1) P1:main/index.tsno longer constructsLocalExecutionor passes it to the runner, so a built app refuses alocalTargetbefore writing it and ends a persisted target's turn with a notice, without starting a child or routing to a cloud backend; no flag or renderer option reopens it. The module and runner seam stay for fixtures and the opt-in native checks until LM-5 (#380) qualifies redirect handling and managed-policy precedence (AC6). (2) P2:CodexBackendomitsmodel_reasoning_summary="detailed"from a local child's arguments and from thethread/start/thread/resumeconfig whenconfiguration.localis set; cloud instances are unchanged. Regressions:local-execution-wiring.test.ts, the extended runner refusal test and the execution test's argument/config assertions (RED on the import and the argument; GREEN after). Build, typecheck, the four focused suites (55/55), the full desktop unit suite (2,029 tests, 2,010 passed, 0 failed, 19 skipped), core/site/store/worktree suites and the re-run codex-cli 0.164.0 wire check all pass; repair evidence. The inherited credential fixes landed in #418.What runs. A thread with a
localTargetruns on its owncodex app-serverchild (engine/local-models/execution.ts): a private empty 0700 home so none of~/.codex(config, plugins, MCP servers, sign-in) is read; an environment allowlist withNO_PROXY=*and no proxy variables; the connection token only inMODEX_LOCAL_TOKEN, named byenv_keyand excluded from the tool shell; providermodex_localat<origin>/v1on the Responses wire, retries off.CodexBackendgains alocalseam: the provider and model are named on everythread/startandthread/resumewithallowProviderModelFallback=false, Codex's reply must confirm both before any content is sent, a handle the child did not open is refused, and a stale sign-in is never retried.Admission (
runner.ts). Selection validates the exact connection revision and context window, pins backend/model, turns Auto and Plan off, drops the native handle and account binding. Local turns carry no account and skip Auto, Jev, titles, follow-ups, the credit fallback and the note/knowledge tools; Plan, images and a build without local execution end the turn with a notice and never route to a cloud backend. Settings are locked while a local turn runs;localTarget: nullreturns to the default Codex model with a fresh handle; a paired desktop's patch cannot carry a target. One turn at a time per loopback port; force-stop ends only the thread's child; delete and quit remove the child and its home.Reconciliation. #401 (superseded by #403, behind main) and the stacked draft #407 were closed with comments; #407's main-side design is carried here, re-derived against today's runner and backend. Its renderer controls are LM-4's (#381).
Checks. RED: tests fail to build against main (22 errors). GREEN: build and
pnpm typecheckpass;codex-backend+local-model-execution+runner-local-models: 54/54; full desktop unit suite: 1,976 tests, 1,957 passed, 0 failed, 19 skipped (17 usual + the 2 opt-in native checks). The installed codex-cli 0.164.0 ran a real shell read through this path against a scripted loopback Responses server (MODEX_LM3_WIRE=1): two requests with the bearer token, built-in tools only (noview_image, web search, plugin or MCP tools), sentinel in the second request, token never in model input, reply carries the sentinel. Receipt; review.Open gates. Product admission waits on LM-5 (#380). AC1's live turn against a real local server is Val's:
MODEX_LM3_LIVE=1 MODEX_LM3_URL=… MODEX_LM3_MODEL=… node --test dist/test/local-codex-native.test.js(Modex starts, installs and downloads nothing). Codex's redirect handling and managed-policy precedence (LM-5), runtime qualification (LM-6), UI (LM-4). No merge or release is requested here.🤖 Generated with Claude Code