Skip to content

fix: persist ACP session config in Codex threads (rebase of #337) + keep thread model/effort on resume (fixes #336, fixes #343) - #394

Open
superbiche wants to merge 5 commits into
agentclientprotocol:mainfrom
superbiche:fix/persist-session-config-rebased
Open

fix: persist ACP session config in Codex threads (rebase of #337) + keep thread model/effort on resume (fixes #336, fixes #343)#394
superbiche wants to merge 5 commits into
agentclientprotocol:mainfrom
superbiche:fix/persist-session-config-rebased

Conversation

@superbiche

Copy link
Copy Markdown

This is @gilbertl's #337 rebased onto current main (1.2.0, b51bedf) with original authorship preserved (cherry-pick -x) — offered because #337 has been sitting in a conflicting state while the #336 symptom keeps biting; all credit for the core fix goes to the original PR. Conflict resolutions worth review: the device-code-auth import block (kept both sides), and sendPrompt — main's effectiveParams slash-command rewrite is kept alongside the PR's lazy () => sessionState.agentMode, including the second call site main added in #351 which the original PR predates (it would no longer compile without this).

On top, one commit fixing #343: thread/resume no longer invents modelProvider: "openai" when nothing is configured — the param is omitted, so thread-carried model/effort survive resume; config defaults still apply when the thread carries nothing. The "openai" fallback dated to #139 (May, older app-server); resuming provider-less under ChatGPT auth works today. Regression test included (fails without the fix). Known residual, deliberately out of scope: an explicitly configured model_provider in config.toml still overrides thread values on resume.

Validation: typecheck/build/test green (404 passed); live end-to-end under ChatGPT auth — reasoning_effort set once, three prompts across three separate client reconnect cycles, all three turns ran at the set effort against a different config default (previously impossible from any released version).

Happy for this to be folded into #337 instead if that's preferred — whatever's easiest to land.


@superbiche · contributor · drafted with Claude Fable 5, reviewed before posting; the voice and decisions are mine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants