Skip to content

fix: publish complete configuration updates for session changes - #715

Merged
benbrandt merged 1 commit into
mainfrom
fix/config-change-notifications
Oct 30, 2026
Merged

benbrandt merged 1 commit into
mainfrom
fix/config-change-notifications

Conversation

@benbrandt

Copy link
Copy Markdown
Member

Summary

  • Extend the existing recovery path's before/after configuration comparison into one shared configuration-change helper. Publish a complete config_option_update when the advertised list changes, rather than duplicating publication in individual callers.
  • Use it for session/set_config_option, the legacy mode/model setters, command-driven changes, approved-plan implementation, and provider/recovery model refreshes. The requesting client's duplicate notification is written before the setter response.
  • Do not publish for unchanged lists, rejected changes, the existing JetBrains 2026.1 disabled-config workaround, or a session state superseded by replay.
  • Build setter responses from the current session state after publication. If replay replaces the session while a native settings request is pending, the old request does not advertise stale options in its notification or response.
  • Keep v2 field conversion in the existing conversion layer. Both ACP versions receive full lists, including dependent changes to reasoning choices and removal of fast mode after a model switch.

Checked against ACP commit 854e6003. docs/protocol/v2/session-config-options.mdx:375-379 recommends configuration notifications whenever configuration changes, including changes requested through session/set_config_option, with the requesting client's duplicate before its response. Responses and notifications replace the complete list.

Scope

Based on the merged #714, not stacked on its feature branch. No review/checklist Markdown file is added. This covers configuration changes owned by the adapter; it does not add new native settings-event synchronization or change active-resume environment replacement. Remaining session/entity metadata reconstruction is separate.

Validation

  • npm run typecheck
  • npm run build
  • npm test: 2,332 passed, 39 skipped
  • npm run test:integration: all 19 passed
  • npm run codex-test -- -p "Reply with exactly OK. Do not use tools or modify any files." -o summary: completed with end_turn
  • git diff --check
  • Opt-in E2E tests were not run.

New wire tests cover notification-before-response ordering for every root config option on both protocol versions, complete dependent model changes, no-op and rejected/native-failure cases, one notification for /plan, legacy setters, and replay replacing the session during a pending native settings request. Existing command, plan, provider/recovery, and child configuration tests remain passing without snapshot updates.

@benbrandt
benbrandt merged commit b7ac0bf into main Oct 30, 2026
6 checks passed
@benbrandt
benbrandt deleted the fix/config-change-notifications branch October 30, 2026 19:07
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.

1 participant