Skip to content

fix: persist ACP model and reasoning effort across turns and resume - #394

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

fix: persist ACP model and reasoning effort across turns and resume#394
superbiche wants to merge 8 commits into
agentclientprotocol:mainfrom
superbiche:fix/persist-session-config-rebased

Conversation

@superbiche

@superbiche superbiche commented Aug 12, 2026

Copy link
Copy Markdown

This refreshes @gilbertl's #337 onto current main (v1.7.0), preserving the original commits and authorship while resolving the conflicts. The scope is intentionally narrower than the earlier revision: model and reasoning-effort persistence only; agent-mode behavior remains identical to upstream main.

What changes:

  • model and reasoning_effort configuration updates are persisted with thread/settings/update and acknowledged only after Codex accepts them. A setting therefore remains effective for subsequent turns and process reconnects until changed.
  • Resume, load, and fork no longer invent modelProvider: "openai" when no provider is configured. Omitting it lets the Codex thread retain its stored model and reasoning effort.
  • Regression coverage exercises persistence across an adapter restart and session load.

Validation on the final candidate (0275316):

  • npm run typecheck: pass
  • npm test: 486 passed, 27 skipped
  • npm run bundle:all: all six platform bundles pass
  • npm run build: pass
  • Live ChatGPT-auth reconnect oracle: selected effort changed highlow; after two independent adapter restarts, both resume and load reported low
  • Independent deep review: three rounds, final PASS; agent-mode persistence was removed during review instead of adding concurrency complexity unrelated to this fix

Known boundary: concurrent ACP callers can still race a setting update against a prompt already in flight. The deployed acpx flow is sequential and waits for the setting acknowledgement before sending the next prompt.

Transport-level effective-config receipts need an authoritative app-server receipt, and per-invocation effort for one-shot acpx exec belongs in acpx; neither is bundled into this adapter fix.

Fixes #336.
Fixes #343.


@superbiche · user · drafted with Codex (GPT-5).

gilbertl and others added 8 commits August 12, 2026 20:01
…onfig-rebased

# Conflicts:
#	src/CodexAcpClient.ts
#	src/__tests__/CodexACPAgent/CodexAcpClient.test.ts
#	src/__tests__/CodexACPAgent/session-config-options.test.ts
@superbiche superbiche changed the title fix: persist ACP session config in Codex threads (rebase of #337) + keep thread model/effort on resume (fixes #336, fixes #343) fix: persist ACP model and reasoning effort across turns and resume Aug 31, 2026
@superbiche

Copy link
Copy Markdown
Author

This conflicted branch has been superseded by #457, rebased onto current main / v1.8.0 with the persistence, prompt-serialization, resume/load/fork, and regression-test updates intact.

I left this PR open rather than rewriting its published history; maintainer review should continue on #457.


@superbiche · user · drafted with Codex (GPT-5).

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

Labels

None yet

Projects

None yet

2 participants