Skip to content

[codex] Fix commitments extractor model selection#75347

Merged
clawsweeper[bot] merged 1 commit intomainfrom
codex/fix-commitments-extractor-model
May 1, 2026
Merged

[codex] Fix commitments extractor model selection#75347
clawsweeper[bot] merged 1 commit intomainfrom
codex/fix-commitments-extractor-model

Conversation

@vignesh07
Copy link
Copy Markdown
Contributor

Summary

Fixes #75334.

The commitments hidden extractor now resolves the owning agent/default model with the same configured model resolver used by normal agent runs before it calls the embedded PI runner. That preserves openai-codex/gpt-5.5 and other configured provider/model refs instead of letting the embedded runner fall back to direct openai/gpt-5.5.

The extractor also opens a short agent-scoped cooldown after terminal model/auth failures, so a bad config does not repeatedly spend hidden extraction attempts or spam the same background error. The cooldown only suppresses the affected agent; other agents can still enqueue extraction.

Root Cause

defaultExtractBatch called runEmbeddedPiAgent without provider or model, so the embedded runner used its static defaults. On OAuth-only OpenAI Codex setups, that became direct OpenAI and produced repeated missing API key errors.

Validation

  • pnpm test src/commitments/runtime.test.ts src/commitments/extraction.test.ts src/commitments/commitments-full-chain.integration.test.ts
  • pnpm test src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts
  • pnpm exec oxfmt --check --threads=1 src/commitments/runtime.ts src/commitments/runtime.test.ts
  • pnpm check:changed

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 1, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

Codex review: passed for ClawSweeper automerge.

What this changes:

The PR updates hidden commitments extraction to resolve and pass the configured agent/default provider and model, adds an agent-scoped cooldown for terminal auth/model failures, adds runtime regression tests, and records the fix in the changelog.

Automerge follow-up:

No repair job is needed: this automerge-opted PR has no review findings, is mergeable, and should be handled by exact-head CI plus the configured automerge path.

Security review:

Security review cleared: The diff only changes model routing, local cooldown state, tests, and changelog text; it does not add dependencies, workflows, secret access, or new external code execution.

Review details

Best possible solution:

Land this focused commitments-runtime fix after exact-head checks and mergeability are accepted, then let it close the linked bug; keep broader background retry or circuit-breaker policy work separate.

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

Yes. The linked bug provides concrete config and log steps, and current main gives a static reproduction: enable commitments with agents.defaults.model.primary = openai-codex/gpt-5.5; the extractor omits provider/model and the embedded runner defaults that omission to direct OpenAI.

Is this the best way to solve the issue?

Yes. Reusing resolveDefaultModelForAgent inside defaultExtractBatch and passing the resolved provider/model is the narrowest maintainable fix for the default-model regression; the cooldown remains commitment-local and does not change shared background-run policy.

What I checked:

  • Current main omits extractor model routing: defaultExtractBatch calls runEmbeddedPiAgent with session, config, prompt, and safety flags but no provider or model, so the configured agent/default model is not carried into hidden extraction on current main. (src/commitments/runtime.ts:179, bd20f8e07e92)
  • Embedded runner defaults explain the reported fallback: When embedded runs receive no provider/model, runEmbeddedPiAgent initializes provider and model from DEFAULT_PROVIDER and DEFAULT_MODEL, matching the direct OpenAI fallback reported by the linked bug. (src/agents/pi-embedded-runner/run.ts:404, bd20f8e07e92)
  • Existing resolver is the right local seam: resolveDefaultModelForAgent already resolves the effective agent/default configured model before falling back to global defaults, so using it avoids new model-selection policy. (src/agents/model-selection.ts:200, bd20f8e07e92)
  • PR applies the narrow runtime fix: The PR diff imports resolveDefaultModelForAgent, resolves modelRef in defaultExtractBatch, and passes provider: modelRef.provider plus model: modelRef.model to runEmbeddedPiAgent. (src/commitments/runtime.ts:211, 8ffa3a8291e3)
  • Regression tests cover model routing and cooldown: The PR adds a runtime test asserting openai-codex/gpt-5.5 is split into provider: openai-codex and model: gpt-5.5, plus a test that terminal auth/model failures suppress only the affected agent until cooldown expiry. (src/commitments/runtime.test.ts:153, 8ffa3a8291e3)
  • Docs support provider-qualified model refs: The config docs specify model.primary as provider/model and explicitly cite openai-codex/gpt-5.5 for Codex OAuth, while the OpenAI provider docs describe openai-codex/* as the Codex OAuth subscription route. Public docs: docs/gateway/config-agents.md. (docs/gateway/config-agents.md:371, bd20f8e07e92)

Likely related people:

  • vignesh07: Recent main history shows Vignesh authored the immediately preceding commitments runtime/test safety work, and the changelog credits this area to @vignesh07. (role: recent commitments maintainer; confidence: high; commits: b277ae3f4c40; files: src/commitments/runtime.ts, src/commitments/runtime.test.ts, src/commitments/commitments-full-chain.integration.test.ts)
  • steipete: Current blame and recent log history tie Peter to the embedded runner defaulting behavior, resolveDefaultModelForAgent, and adjacent external credential discovery work. (role: model-routing and embedded-runner maintainer; confidence: medium; commits: b40c679630d6, 90419df6633f; files: src/agents/pi-embedded-runner/run.ts, src/agents/model-selection.ts, src/commitments/runtime.ts)
  • pashpashpash: Recent adjacent history changed embedded runner state handling for Codex app-server isolation, which is near the provider/model runtime boundary touched by this PR. (role: adjacent embedded runtime contributor; confidence: low; commits: 027ea5f08bd9; files: src/agents/pi-embedded-runner/run.ts)

Remaining risk / open question:

  • No local tests were run because this was a read-only review; exact-head CI/check-run data and the PR’s stated validation should remain the merge gate.

Codex review notes: model gpt-5.5, reasoning high; reviewed against bd20f8e07e92.

@vignesh07
Copy link
Copy Markdown
Contributor Author

@clawsweeper automerge

@vignesh07 vignesh07 marked this pull request as ready for review May 1, 2026 01:43
@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 1, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

🦞🦞
ClawSweeper automerge is enabled for this PR.

I added clawsweeper:automerge and asked ClawSweeper to review this head. If ClawSweeper emits a repair marker or requests changes, I will repair/rebase the branch and ask for another review, up to the configured round limit.

Draft PRs stay fix-only until GitHub marks them ready for review. A maintainer can pause this with /clawsweeper stop.

@clawsweeper clawsweeper Bot merged commit 38da2ac into main May 1, 2026
119 of 120 checks passed
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 1, 2026

🦞🦞
Thanks, ClawSweeper. ClawSweeper merged this PR after the passing review.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=8ffa3a8291e3cbb54abae08a81650040e1b1b92d)
Merge status: merged by ClawSweeper automerge
Merged at: 2026-05-01T01:57:22Z

The automerge loop is complete.

@clawsweeper clawsweeper Bot deleted the codex/fix-commitments-extractor-model branch May 1, 2026 01:57
lxe pushed a commit to lxe/openclaw that referenced this pull request May 6, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: commitments extractor uses direct OpenAI instead of configured openai-codex model, causing repeated background auth failures

1 participant