fix(agent): align effort schema with provider-specific capabilities - #234
fix(agent): align effort schema with provider-specific capabilities#234lucas77778 wants to merge 6 commits into
Conversation
Co-authored-by: Chenyu Lu <chenyu@arcbox.dev>
CODE-330 fix(agent): align effort schema with provider-specific capabilities
ProblemThe normalized effort contract is a single global enum and the UI keeps provider effort choices in static tables. That contract no longer matches the pinned Codex 0.144.1 provider:
A configured Codex Root causeEffort vocabulary and availability are modeled globally, although they are provider- and model-specific. The adapter already reads Fix
Acceptance
EvidenceVerified directly against the repository-pinned |
Greptile SummaryThis PR aligns reasoning-effort handling with each provider and model. The main changes are:
Confidence Score: 5/5This looks safe to merge. Optional or empty Codex catalogs no longer block session startup. Pending model selections survive stale updates and process recovery. No blocking issues were found in the changed code.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Reviews (5): Last reviewed commit: "fix(agent): resolve effort review blocke..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-authored-by: Chenyu Lu <chenyu@arcbox.dev>
Co-authored-by: Chenyu Lu <chenyu@arcbox.dev>
Co-authored-by: Chenyu Lu <chenyu@arcbox.dev>
Zerlight
left a comment
Zerlight
left a comment
There was a problem hiding this comment.
Requesting changes for two correctness issues:
- The inline comment covers the live pending-selection lifetime.
- The static fallback capability metadata is not used by draft submission.
new-session-surface.tsx:169-174resolvesmodelOptiononly fromdynamicModels, while this PR adds model-specific fallback capabilities inagent-models.ts:90-108. Ifmodel/listis unavailable, a remembered Solultraselection can survive after selecting Luna and is submitted even though the fallback table excludes it, causing the first turn to fail. Resolve the selected model from the dynamic catalog orAGENT_MODEL_OPTIONS[provider]consistently before calculatingconstrainedEffort.
The pinned 0.144.1 protocol shape and CI both check out; these are the remaining blockers.
|
Addressed the second review blocker in cd7cb81. |
# Conflicts: # packages/foundation/schema/src/wire/message.ts # packages/foundation/schema/tests/contract/wire/agent.test.ts # packages/host/agent-adapter/src/__tests__/codex-shell.test.ts # packages/host/agent-adapter/src/native/codex/adapter.ts # packages/presentation/ui/src/shell/new-session-surface.tsx
Zerlight
left a comment
Zerlight
left a comment
There was a problem hiding this comment.
Requesting changes for one remaining correctness issue and one documentation inconsistency.
The previous model-pending fix and the static fallback fix look correct, but the equivalent resume-confirmation path for pendingEffort is still incomplete. A focused adapter regression reproduces the stale reflection deterministically.
Verification on this head: the full test suite passed (244 files, 1,965 tests; 3 files and 5 tests skipped), pnpm check:ci passed, git diff --check passed, and all GitHub checks are green.
| if (this.effort !== undefined) { | ||
| this.assertEffortSupported(this.effort, this.model ?? model, !this.modelCatalogAvailable); | ||
| } | ||
| if (this.pendingEffort !== undefined) return; |
There was a problem hiding this comment.
[P1] Clear an effort confirmed by thread/resume
BaseAgentAdapter.start() calls onSetEffort() before onStart(), so resuming with a remembered effort such as high sets pendingEffort before thread/resume. If the resume response already reports high, this early return leaves the marker set. The app-server only emits thread/settings/updated when the effective settings actually change, so another matching high notification is not guaranteed. A later authoritative correction to medium then reaches the pending-effort branch and is discarded indefinitely, leaving the engine/UI cache at high.
Resolve the response effort (or model default) before returning and clear pendingEffort when it matches, paralleling the pendingModel handling above. I reproduced this with the existing TestCodex fixture: after a resume response with high, a medium settings update emitted no effort-update, both inside and outside the sandbox.
| - **Approval-policy axis** (three tiers; claude-only ids like `plan`/`auto` reject): `default`→untrusted + workspace-write, `acceptEdits` (initial)→on-request + workspace-write, `bypassPermissions`→never + danger-full-access. Applied per `turn/start` — next-turn semantics, same channel as model/effort; nothing can alter an in-flight turn. | ||
| - **config.toml contract**: `codexConfiguredSandbox()` (`codex/config.ts`; exercised via a throwaway `CODEX_HOME` in `codex-config.test.ts`) reads the active profile's or top-level `sandbox_mode`. Until the user EXPLICITLY picks a tier, a configured sandbox is never overridden — thread/turn requests omit their sandbox override so codex's own resolution wins (NEVER silently loosen a stricter read-only); the preset's approval posture still rides, which is safe now that approvals are answerable. An explicit pick applies the preset exactly, config.toml notwithstanding. Honoring config.toml `approval_policy` stays an unfinished follow-up. | ||
| - **set-model / set-effort** are stored and sent as `turn/start` overrides (they stick for subsequent turns). Effort accepts `low`–`xhigh`; `max`/`ultracode` are claude-only and reject. The picker entries are the static UI tables (`agent-models.ts` / `agent-efforts.ts`), each verified live via `turn_context` readback. At startup, the adapter reflects the `thread/start`/`thread/resume` response's model and configured effort; a null effort resolves through that model's `model/list.defaultReasoningEffort` without pinning it as an override. `thread/settings/updated` is the ongoing effective model/effort authority. The rest of the dynamic catalog remains deliberately unused (the CODE-104 attempt was cancelled). | ||
| - **set-model / set-effort** are stored and sent as `turn/start` overrides (they stick for subsequent turns). Effort availability comes from each `model/list` entry's `supportedReasoningEfforts`: Sol/Terra advertise `low|medium|high|xhigh|max|ultra`, while Luna omits `ultra`; `minimal` is outside LinkCode's normalized vocabulary and Claude's distinct `ultracode` always rejects. The adapter publishes that per-model catalog both before session start and on the live event stream, and validates startup/live selections against the selected model. If optional `model/list` discovery is unavailable, validation defers to Codex instead of blocking session start. At startup, it reflects the `thread/start`/`thread/resume` response's model and configured effort; a null effort resolves through that model's `model/list.defaultReasoningEffort` without pinning it as an override. `thread/settings/updated` reflects effective provider state unless it conflicts with a newer pending next-turn pick. Switches apply from the next turn, not mid-turn. |
There was a problem hiding this comment.
[P2] Keep the capability matrix in sync
This paragraph now documents model-specific max and ultra support, but the capability matrix below still says Codex supports only low–xhigh. Please update the Codex row so the package's reference documentation agrees with the adapter and UI.
Summary
ultralevel and advertise per-model effort capabilities/defaultsmodel/listmetadata for pre-session and live catalogs, validating startup and live model/effort selections against the selected modelultraseparate from Claude'sultracode, update effort filtering/fallbacks, and preserve pending picks across stale settings notificationsVerification
pnpm test(210 files, 1,634 tests)pnpm check:cigit diff --checksupportedReasoningEffortsprotocol shape against@openai/codex 0.144.1Linear: CODE-330