Capture resolved model/effort/sandbox in job records#506
Open
cubicj wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Job records never capture the model, reasoning effort, or sandbox a job actually ran with (#481). Background task jobs persist the raw CLI request, but those fields are
nullwhen flags are omitted; foreground tasks and review jobs record nothing at all. The only place the resolved values exist today is the Codex rollout file, and joining a job record to its rollout fails in practice: date directories use local time while job timestamps are UTC, and rollout filenames carry a session id that does not match the job'sthreadId(both hit independently in the issue thread).Change
The app-server already reports what it resolved:
thread/startandthread/resumeresponses echomodel,modelProvider,reasoningEffort, and the fullsandboxpolicy with config.toml fallback applied, and the plugin currently discards everything butthread.id. This PR captures that echo instead of adding any new resolution logic, so it stays truthful regardless of how the pending override-mechanism discussions (#471, #408, #476, #482) land.lib/codex.mjs:runAppServerTurnandrunAppServerReviewbuildresolved = { model, modelProvider, reasoningEffort, sandbox }from the thread response, attach it to the existing "Thread ready" progress event, and include it in the returned result.turn/startresponses carry no effort field, so an explicit per-turn--effortis overlaid ontoresolvedonly after theturn/startrequest is accepted — a rejected or failed turn never records a requested effort as if it had run.lib/tracked-jobs.mjs: the job progress updater patchesresolvedinto the job file and state index mid-run, the same way it already handlesthreadId, so failed and cancelled jobs still record what they ran with. The completion write addsresolvedexplicitly because it spreads the in-memory running record.codex-companion.mjs: each execution object liftsresult.resolvednext tothreadId/turnId; all job kinds sharerunTrackedJob, so foreground tasks, background workers, native review, and adversarial review are covered without per-path changes.Job records gain one nested field, next to the existing raw
requestfields, so both layers stay visible — requested versus server-confirmed:Human-readable output is unchanged: renderers read job fields selectively, the progress-event patch rides the same write the
threadIdupdate already performs, and no log line formats change.status --jsonandresult --jsongain the field additively.Verification
npm test: 92/92 passing. New coverage assertsresolvedin the persisted job JSON for native review, adversarial review, resumed tasks, and the detached background worker (checked mid-run, while stillrunning); an explicit--effortoverriding the thread echo on success; and aturn/startfailure case with an explicit--effortproving the failed record keeps the thread echo rather than the never-applied request.npm run build(typecheck viatsconfig.app-server.json): clean.git diff --checkagainst the base commit: clean.thread/start/thread/resumeresponses now mirror the real app-server's resolved echo (shape confirmed against the generatedThreadStartResponse/ThreadResumeResponsetypes and a live JSON-RPC probe against codex-cli 0.144.1).Related Issues
Fixes #481