Repository navigation
Let fresh Codex starts through the session proxy - #2215
Conversation
The stock TUI's thread/start carries dynamic tool JSON Schemas that describe a threadId property. The proxy read those schema keys as thread identities and refused every fresh launch. Skip schema payloads and null optional IDs, keep refusing malformed or missing identities on thread actions, and take the ownership lease from the thread ID the start returns. Refs #2213
|
Review prompt for PR #2215 at |
|
Review of record: OpenAI, run by the Supervisor from its iTerm session (route on codex-skills#2069), at head Medium — skills/supervisor/scripts/codex_session_proxy.py:229: Replacing |
A thread/start whose params name an existing thread acquired that lease, and taking the new thread's lease replaced the guarded set, so the request's lease was never released. Hold the start lease as its own references and release both. Refs #2213
|
Review result: weighed Reviewer: OpenAI, review of record, run by the Supervisor at Acted on (medium, reproduced): a The same commit also simplifies the pending-request unpack (same behavior). This clears the inspection warning on that line. Declined: none. Unresolved: none. Since the review, the only change is that commit. The 429 tests in the five affected modules pass again. Inspection of the changed files is RED only for the existing ParamSpec warnings on |
|
Review prompt for PR #2215 at new head |
|
Review of record: OpenAI, run by the Supervisor from its iTerm session (route on codex-skills#2069), at head none |
|
Review result: weighed Reviewer: OpenAI, review of record, run by the Supervisor at The earlier medium finding (start leases replacing request leases) was acted on in |
|
Ready for the merge train Head: 7a3c0e0 |
|
Launchplane completed the merge-train step for this pull request.
Launchplane finished the merge-train step for this pull request. Launchplane manages this comment and will update it as the train moves. |
…3c5994691e0ecd0a: merge PR #2215
Every fresh Codex launch through the Supervisor launcher has failed since #2148 with
thread/start failed: native thread identity is unavailable. Existing sessions kept running.Why
The real Codex 0.163.0 TUI was captured against a throwaway app server. Its
thread/startsends no thread ID. It does senddynamicTools, and their JSON SchemainputSchemadeclares athreadIdproperty.thread_targets()walked into that schema, saw athreadIdkey whose value was a dict, and refused. The protocol has two more fields of the same kind:turn/startoutputSchemaand the server elicitation'srequestedSchema. Several requests (apps/list,mcpServerStatus/listand others) also allowthreadId: null.What changed
thread_targets()skipsinputSchema,outputSchemaandrequestedSchema, and treats a null optional ID as no target. Wrong-typed and empty IDs still refuse. Thread and turn actions with no ID still refuse at the existing request check.thread/startreply arrives, the ingress takes the ownership lease and durable action barrier on the returnedthread.idbefore forwarding the reply. If the ID is missing or the lease fails, the client gets an RPC error instead. This matches how resume metadata failures are reported.helpers.mddescribes the fresh-start rule.Every ownership guarantee from #2148 stays as it was: resume, fork, queue, approval and admission paths are unchanged.
Verification
test_fresh_start_owns_its_returned_thread_and_malformed_identities_refusedrives a fresh start through the real ingress and the suite's fake Unix app server. The start carries the TUI's dynamic-tool schema shape. The test then checks that the returned thread has its barrier and a free lease, that aturn/startwith anoutputSchemapasses, and that a start reply with no usable ID errors. It also checks that five malformed or missing identities refuse without reaching the server. Without the fix it fails at the start (KeyError: 'result').thread/startpayload yields no targets with the fix; the current main refuses it.test_codex_session_proxy(32),test_codex_session(8),test_github_plan_sessions(42),test_gh_plan_claim(253),test_release_session_claims(94).validate-skill-repo,validate-public-safetyandvalidate_role_wordspass.check_test_qualityshows only the three existing unrelated warnings.acquire/release, unresolvedsys.pathimports, ParamSpec warnings, existing doc spelling. None are on the new test or doc lines.The remaining finish line is one real fresh launch under
tmux -L supervisorafter this lands.Refs #2213