Skip to content

fix(runtime): retry the startup model probe once when the provider is unreachable - #2672

Merged
freeqaz-openai merged 2 commits into
mainfrom
lows-1017c-1618
Oct 17, 2026
Merged

freeqaz-openai merged 2 commits into
mainfrom
lows-1017c-1618

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Finding 1618: with the model provider unreachable (connection refused), the dedicated Codex startup model check ran 30 s to its timeout and was held at once as error-event, with no second attempt, although a failed connection is meant to keep its single retry.

The pinned Codex build (0.163.0-alpha.2) does not fail the turn on a failed connection. It prints

{"type":"error","message":"Reconnecting... waiting for network (Connection failed: error sending request)"}

and keeps waiting. A refused port, an unresolvable host and a failed TLS connection all print exactly this (checked by running the pinned binary against http://127.0.0.1:1, http://no-such-host.invalid and https://127.0.0.1:9). codexTransientFailure vetoed every Reconnecting... notice, and the recovered-stream rule only accepts the bounded n/m ... stream disconnected shape, so the first notice was held as fatal.

Change

  • codexTransientFailure keeps its veto on Reconnecting notices except one: a notice that is exactly Reconnecting... waiting for network (Connection failed: ...). That one is transient, so the probe takes the existing single retry inside the same 61 s budget (no new caps). Credential markers, rate limits, provider 4xx, certificate and NXDOMAIN rules still apply to it first.
  • Comments updated. The diagnostics flow doc says an unreachable provider holds error-event-attempt2 about a minute after the check starts.

The retry-guard hunks of #2487 (the providerTools === "none" error-event guard and the timeout retry) are untouched; git merge-tree against its head is clean.

Verification

Risks

  • Internal review: approve with nits (applied). With Credential Proxy Implementation #2487's providerTools: "none" mode, an unreachable provider would still take this transient retry; nothing reached the provider, so that looks safe, but whoever lands Credential Proxy Implementation #2487 should know.

  • Each attempt against an unreachable provider still waits out its 30 s limit, so such a deployment fails after about 61 s instead of 30 s.

  • A counted Reconnecting... n/m (Connection failed: ...) shape is not printed by the pinned build and stays held; a Codex bump that changes the network-wait wording falls back to today's behavior (held at once).

… unreachable

Codex reports a provider it cannot connect to as a Reconnecting network-wait
notice and keeps waiting, so the startup probe ran to its timeout and held the
notice at once as a fatal error event. Treat a network wait whose cause is a
failed connection as transient, so it gets the existing single retry within the
61 s budget. Other Reconnecting notices the recovered-stream rule refuses stay
held, and credential markers still veto. Finding 1618.
@freeqaz
freeqaz requested a review from a team as a code owner October 17, 2026 02:23
@clawsweeper

clawsweeper Bot commented Oct 17, 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.

… into the veto comment

Review nits on finding 1618: a network wait with text after its cause stays
held, the boundary shapes are labelled as such, and the transient comment
names the network-wait exception in its first sentence.
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 17, 2026 02:28 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 17, 2026 02:28 — with GitHub Actions Inactive

@freeqaz-openai freeqaz-openai 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.

I reproduced the missing retry with the actual pinned codex-cli 0.163.0-alpha.2, then observed the corrected behavior through the generated entrypoint and its runtime-status endpoint.

Same confirmed-refused loopback endpoint Probe attempts Final status; readiness
Parent 7dcd2b67 1 error-event after 30.2 s; never ready
Changed executable behavior 2 error-event-attempt2 after 61.2 s; never ready

These are probe attempts, not a count of connections Codex may try inside one attempt. This was an existing runtime image carrying the pinned CLI, source-generated entrypoints, a synthetic credential, loopback-only networking and no external provider or Kubernetes deployment. The first generic offline control produced an ordinary timeout, which already retries; selecting an explicitly refused endpoint through the real configuration generator made the regression observable. The container exited and was removed.

At final c46fca08, the generated program differs from what I exercised by five comment lines only. All 127 final conformance tests passed; pairing the parent implementation with the new tests failed specifically on the unreachable-provider retry and held-cause assertions. Required CI and Image Model Probes succeeded on this exact head. This supports the bounded retry and diagnostic, not a claim that an arbitrary provider outage is a refused connection or that an unreachable model became available.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. labels Oct 17, 2026
@clawsweeper

clawsweeper Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

The PR gives Codex’s exact failed-connection network-wait notice the existing single startup-probe retry.

Example: The pinned Codex CLI probes a confirmed-refused loopback provider endpoint.

  • Before: One probe attempt ends with error-event after about 30 seconds.
  • After: Two probe attempts end with error-event-attempt2 after about 61 seconds if the endpoint remains unreachable.

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A focused repair with recorded native-runtime before/after evidence, meaningful regression coverage and no actionable finding.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The collaborator’s recorded container run exercises the source-generated runtime entrypoint, actual codex-cli 0.163.0-alpha.2, real refused-loopback transport and runtime-status endpoint, showing the corrected two-attempt held failure; conformance separately covers recovery on attempt two, while live Kubernetes recovery is not claimed.
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 temporary provider connection failure can permanently fail an Agent deployment without its intended second startup probe.
Reason: The maintainer-authored direction and collaborator approval support restoring the existing retry contract for an observed native message.

Merge readiness

✅ Ready for maintainer review

Keep open: this maintainer-authored PR repairs a verified gap in the existing startup retry policy, with no actionable defect found.

Priority: P2
Reviewed head: c46fca08c9a6af3cbd6b27456c9706725925e017

Before merge

None.

Findings

None.

Agent review details

How this fits together

The shared dedicated Codex runtime entrypoint receives admitted model configuration, runs the native authentication probe, and either starts the app server or exposes a held startup failure through runtime status.

flowchart TD
  A[Admitted Agent configuration] --> B[Dedicated runtime entrypoint]
  B --> C[Native Codex model probe]
  C --> D[Failure classification]
  D --> E[One bounded retry]
  E --> C
  D --> F[Held failure status]
  C --> G[Ready app server]
Loading

Technical review

Best possible solution:

Land the narrow classifier correction while preserving the existing retry budget and deterministic-failure exclusions.

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

Current main visibly rejects the exact network-wait message before transient classification; the linked approval independently records the parent failure using the actual pinned CLI and generated entrypoint.

Is this the best way to solve the issue?

The change repairs the classification at its shared production owner, reuses the existing retry mechanism, and preserves the exclusions established by earlier fixes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 0e997c497afa.

Provenance checked

  • Transient startup-probe classification changes intended behavior with a stated reason (04d64a9: Retry transient provider and connection failures once while rejecting unrecognized Reconnecting notices.)
  • Deterministic provider and configuration exclusions keeps the original intent (30a2e68: Avoid another potentially billed probe for failures known to repeat, while allowing opaque connection failures their retry.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: A specific unreachable-provider message misses the intended startup retry; the repair has a limited runtime scope and retains the existing failure bounds.
  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: A specific unreachable-provider message misses the intended startup retry; the repair has a limited runtime scope and retains the existing failure bounds.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster 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.

Reviewed October 16, 2026, 10:40 PM ET / October 17, 2026, 02:40 UTC.

@freeqaz-openai
freeqaz-openai merged commit c5fac47 into main Oct 17, 2026
52 of 54 checks passed
@freeqaz-openai
freeqaz-openai deleted the lows-1017c-1618 branch October 17, 2026 02:42

This branch was previously deployed

1 inactive deployment
integration-qa-pr — c46fca08 Deployed Oct 17, 2026 by freeqaz via QA advisory (compose) #1664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor 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