Skip to content

Disable unbounded connection retries for protected Codex launches - #3310

Merged
freeqaz merged 3 commits into
mainfrom
freeqaz/stop-protected-codex-waiting-to-retry
Oct 23, 2026
Merged

freeqaz merged 3 commits into
mainfrom
freeqaz/stop-protected-codex-waiting-to-retry

Conversation

@freeqaz-openai

Copy link
Copy Markdown
Collaborator

With request and stream retry limits set to zero, Codex can still wait after an initial connection failure and send a protected model request when the endpoint becomes available.

For newly admitted Kubernetes revisions using dedicated Codex with protected inference, disable unbounded_connection_retries in the authentication probe and app-server. This covers inference routes and separately admitted model credentials alongside Git. Existing revisions retain their exact launch commands; older workers reject the new admission marker. Roll out the updated controller and workers before creating a new Agent revision to pick up this behavior.

Validation used the shipped Codex binary with a controlled local endpoint. The default configuration waited and sent after the endpoint became available; the new setting terminated without a request to the later listener. An already available endpoint succeeded. A driver-produced launch also completed two conversation turns with continuity. Relevant integration tests, type checking, lint and formatting passed. These checks do not establish behavior for other network failures or a real deployment.

Stamp a private Compute admission fact for new protected Codex inference revisions and disable unbounded connection retries in the startup probe and app-server launch. Preserve legacy launch bytes and reject incompatible admission markers.

Extend the local enrollment integration to cover admission, native arguments, both launch forms, and refusal controls.
@freeqaz-openai
freeqaz-openai requested a review from a team as a code owner October 23, 2026 13:26
@freeqaz-openai freeqaz-openai added the agent-created PR opened by an AI agent; does not identify the reviewer or approver. label Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
Workspace rendering does not need to inspect admission. Pass the validated workspace command to new and replacement deployments; ordinary callers retain the existing default.
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR disables native connection waiting in the probe and app-server for newly admitted protected dedicated Codex revisions.

Example: A protected Codex launch encounters an unavailable Responses endpoint that later starts listening.

  • Before: The reported native comparison recovered and sent one POST despite zero request and stream retry budgets.
  • After: The new launch exited unsuccessfully and sent zero POSTs during the observation; the healthy control sent one POST.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-verified repair with a controlled native fault comparison and resolved prior finding; full deployment behavior remains outside the supplied proof.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The author's current-head native comparison exercises generated protected startup with shipped Codex, signed fixture enrollment, and a controlled TLS Responses endpoint: delayed-listener recovery sends one POST on the retained command and zero on the new command, while the healthy control succeeds; direct/workspace launches and conversation continuity are also reported, without claiming real-deployment or all-network-failure coverage.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A protected model request can remain pending after connection failure and be sent when the endpoint later becomes available.
Reason: The maintainer-selected direction closes a demonstrated gap in existing retry controls without adding a public setting or changing authorization.

Merge readiness

✅ Ready for maintainer review

The prior documentation finding is resolved, and this review found no remaining actionable defect. Keep this maintainer-authored PR open for normal landing review; CI is still running.

Priority: P2
Reviewed head: ac24eb9c251acf497823040450d2be54c7dea91e

Before merge

None.

Findings

None.

Agent review details

How this fits together

Kubernetes Compute converts immutable Agent revision admission into direct or workspace-node Harness commands, which launch the Codex authentication probe and app-server.

flowchart TB
 A[Agent deployment] --> B[Compute admission]
 B --> C[Stored Agent revision]
 C --> D[Compute launch selection]
 D --> E[Codex authentication probe]
 D --> F[Codex app server]
 E --> G[Responses endpoint]
 F --> G
Loading

Technical review

Best possible solution:

Land the focused launch change with its existing revision boundary and documented controller-and-worker rollout order.

Do we have a high-confidence way to reproduce the issue?

Current main omits the native feature disable despite zero provider retry budgets, and the author reports a controlled native base/head comparison; this review executed no target runtime.

Is this the best way to solve the issue?

The change addresses the native connection-wait owner directly, reuses existing launch templates and admission storage, and preserves immutable commands for existing revisions.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 870cf5d8e1a6.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 975f744 adds apps/controller/src/drivers/compute/kubernetes/index.ts:12123 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 975f74489144; files: apps/controller/src/drivers/compute/kubernetes/index.ts)
  • kevinlin-openai: Raw commit 73c2ef4 adds apps/controller/src/drivers/compute/kubernetes/runtime-entrypoints.ts:757 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 73c2ef49b3a8; files: apps/controller/src/drivers/compute/kubernetes/runtime-entrypoints.ts)

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This corrects connection waiting on the limited protected dedicated Codex launch path.
  • merge-risk: 🚨 compatibility: Older workers reject newly marked revisions; the maintainer explicitly accepts upgrading controller and workers before admitting them.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-23T13:42:16.415Z sha 162c913 :: needs changes before merge. :: [P2] Keep the topology flow within the enforced word limit

Reviewed October 23, 2026, 9:57 AM ET / 13:57 UTC (Revision 2).

@freeqaz-openai

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

Fixed the requested documentation limit and the other red CI lane in ac24eb9c251acf497823040450d2be54c7dea91e. The repository counter now reports 2,470 words (limit 2,500); the documentation checks, site build and search index passed. The workspace renderer now receives the command Compute already selected, so it need not rediscover admission. Both affected conformance files passed locally: 25/25 and 379/379, zero skips. Hosted CI for this head is separate.

Here is the requested native comparison through the generated protected startup, using the shipped Codex 0.163.0-alpha.2, signed fixture enrollment and a loopback TLS Responses endpoint. A small event tap forwarded the probe's actual arguments and output and told the fixture when to open the listener. Each arm ran one probe attempt; the observable was an HTTP POST at the receiver, not an inference from the startup status.

Generated launch When the listener opened Native result OCE probe POSTs received
Base, retained command After Codex reported it was waiting for the network Recovered, exit 0 MODEL_PROBE_FAILED 1
This head, new protected revision After Codex reported the failed turn Failed, exit 1 MODEL_PROBE_FAILED 0
This head, healthy control Before the request Completed, exit 0 READY 1

On the refused arm we kept the listener open through the terminal probe result and another 1.4 seconds, then retired the fixture. This is a finite observation, not a claim about every network failure, redirects, or exactly-once delivery. A separate run exercised the generated direct and workspace-node commands and two turns in the same conversation. No real provider credential or service was involved.

@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
@freeqaz
freeqaz merged commit aac5c62 into main Oct 23, 2026
76 checks passed
@freeqaz
freeqaz deleted the freeqaz/stop-protected-codex-waiting-to-retry branch October 23, 2026 14:16
freeqaz added a commit that referenced this pull request Oct 23, 2026
)

Adds a conformance check that the Kubernetes driver renders the protected Codex launch command from #3310 for admitted protected revisions, with and without a workspace node, and the legacy command for unmarked revisions. Reverting the entrypoint selection or the prepare-time workspace node now fails it. This also guards the line that open #2982 rewrites.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-created PR opened by an AI agent; does not identify the reviewer or approver. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants