Skip to content

feat(config): migrate all remaining writers to patch transactions - #1513

Open
cyhhao wants to merge 27 commits into
masterfrom
fix/config-patch-writers-stage3
Open

feat(config): migrate all remaining writers to patch transactions#1513
cyhhao wants to merge 27 commits into
masterfrom
fix/config-patch-writers-stage3

Conversation

@cyhhao

@cyhhao cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR completes stage 3 of the config write-serialization plan. The core
direction is to make the mutation owner explicit at both boundaries:

  • UI mutation boundary: ApiContext.mutateConfig is the only UI path that
    posts to /api/config. Callers submit field-level mutations or explicit
    platform list operations; Wizard and Messaging flows no longer resubmit
    mount-time or remote full-config snapshots.
  • Server transaction boundary: config_file_lock is the single
    cross-process lock for a complete lock-fresh read/merge/validate/write cycle.
    Existing patch mutators and first-run/QR writers use that boundary, and the
    remaining endpoint preserves its merge-patch API compatibility.

The lock serializes overlapping writers, but it cannot make a stale snapshot
safe. That is why the explicit UI mutation protocol and the server-side
lock-fresh mutators are both required.

What changed

  • Migrated all UI config writers to mutateConfig and centralized payload
    serialization, including Wizard, Settings Messaging, platform/backend
    settings, runtime settings, channels, and workbench controls.
  • Added list-operation support for platforms.enabled and validation that
    rejects unsafe, undefined, conflicting, or object-valued leaf mutations.
  • Added Wizard-specific mutation builders so each step submits only fields it
    owns; finish persistence submits setup/auto-update changes and list ops only.
  • Consolidated server writers on config_file_lock, removing the duplicate
    lock abstraction and narrowing the WeChat QR writer to its token plus an
    explicit platform enable operation.
  • Added a static guard proving the UI has one /api/config mutation boundary,
    plus focused unit, transaction, auth, runtime, and UI regression coverage.
  • Merged the current master into the branch without rewriting history.

Validation

  • Python focused suites: 79 API merge tests, 214 config transaction/memory/API
    tests, and 361 runtime/UI-server/CLI/WeChat tests passed locally.
  • UI focused tests: 20 passed; TypeScript lint and production build passed
    (only existing dependency/chunk-size warnings remain).
  • git diff --check passed.

Review close criteria

The PR is ready to merge only after the pushed head has green CI and no
unresolved actionable review threads. The tracker issue remains open until
that condition is met; no merge is performed by this change.

Dependencies

None.

@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: faf54e4731

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/api.py Outdated
Comment thread core/agent_auth_service.py
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Both P1s fixed in 9dc9518:

P1 stale-decision application: _persist_backend_auth_mode now computes needs_mode_write / needs_marker_write / needs_codex_oauth_cleanup AND the marker-retention decision INSIDE the transaction mutator, from the lock-fresh snapshot (cfg_target), then applies and mirrors the committed result. The lock-outside decisions (and the early no-op path built from them) are gone — an OAuth completion racing a cross-process auth save now decides on current state, so it can no longer leave a stale API-key mode selected or restore an older relay marker over a fresh one. Affected tests now seed the REAL (isolated) config with the api_key pre-state, since the transaction legitimately warrants its write from the file, not the live fake.

P1 full-snapshot UI callers: SettingsPlatformsPage no longer round-trips {...config} — persistEnabled and the enable-after-apply step send only {"platforms": {"enabled": …}}; the platform-card apply sends the card's own section payload + the enabled list (that's ...nextData, the card's buildSubmitData, not the whole config). SlackConfig.buildSubmitData drops the top-level mode: 'self_host' (the settings page doesn't own the deployment mode; the wizard supplies its own mode in buildConfigPayload). UI build green.

602 backend tests + ruff green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dc9518e5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/api.py Outdated
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

P1 agent-section snapshots — fixed in this head: SettingsBackendsPage.persist is now persistBackendField(name, patch) sending only {"agents": {[name]: {enabled}}} for toggles; useBackendRuntime.onSaveRuntime sends only {"agents": {[backend]: {enabled, cli_path}}} and the enable-toggle path sends only the flag — the full agents rebuilds (and the pre-save getConfig round-trips feeding them) are gone, along with the now-unused withoutLegacyDefaultBackend helper. Deep-merge over the lock-fresh base can no longer apply a stale sibling cli_path from these callers. UI build green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d943ad1e9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/api.py Outdated
Comment thread vibe/ui_server.py Outdated
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Both round-4 P1s fixed in 1b65630:

P1 ChannelList snapshot: saveLatestConfig (full configRef round-trip) is replaced by savePlatformFlagPatch, which sends only {[platform]: {[field]: value}} for the toggled flag — its single caller (savePlatformFlag) is the only user of the old saver. The in-memory optimistic update via applyConfig is unchanged.

P1 WeChat QR section staleness: _persist_wechat_qr_credentials now computes the wechat fields AND the enabled-list mutation inside config_lock_transaction from the lock-fresh snapshot (fresh-install seeding preserved via the default-config fallback), so a concurrent wechat/platform save between an earlier read and this write can no longer be overwritten by stale section values.

190 + 122 tests, ruff, UI build + lint all green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b656307d1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/components/settings/SettingsPlatformsPage.tsx Outdated
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Round-5 P1 (stale enabled-list replacement) fixed in 8c054de with a list-operations verb instead of another caller patch:

  • save_config now accepts a reserved __avibe_list_ops payload key: {"platforms.enabled": {"add": [...], "remove": [...]}} mutates the lock-fresh persisted list inside the transaction (deep-merge replaces lists wholesale, so a section patch carrying the whole stale array still clobbers). The verb is popped before stripping/validation and never reaches from_payload.
  • SettingsPlatformsPage toggles (disable-with-confirm, enable-from-tile, enable-after-apply) all send add/remove single-platform operations instead of list snapshots.
  • Pinned by test_save_config_list_ops_merge_against_lock_fresh_base: a toggle computed before another process enabled a platform no longer drops it.

191 + 117 tests, ruff, UI build + lint green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c054dea4b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/api.py
Comment thread ui/src/components/settings/SettingsPlatformsPage.tsx Outdated
Comment thread vibe/ui_server.py Outdated
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Round-6 findings addressed in dd17780 — all three concern the NEW list-ops verb (same-shape per the standing rule), not new snapshot sites:

P1 validation bypass: after the list ops apply, the payload now surfaces the FINAL post-operation platforms section (valid strings), so _platforms_requiring_runtime_credential_validation validates list-op additions exactly like explicit list saves — enabling an uncredentialed platform through the verb is rejected (test_list_ops_enable_requires_credentials).

P1 reconciliation skip: _platform_runtime_fields_changed treats any payload carrying the __avibe_list_ops verb as a platforms edit, so enable/disable toggles still trigger the controller IPC hot reconciliation.

P2 ASGI loop blocking: _persist_wechat_qr_credentials now runs via asyncio.to_thread from the QR poll route (config-lock wait + file/DB work off the event loop).

308 + 71 tests, ruff green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd1778035f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/components/settings/SettingsPlatformsPage.tsx Outdated
@cyhhao

cyhhao commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in 0b6f8ea: the credential-apply save now sends ONLY the card's nextData (its platform section + mode-free fields) — the stale enabled snapshot no longer rides along. Enablement remains expressed solely through the list-operation verb (the subsequent add op mutates the lock-fresh list), so a concurrent platform enablement by another process can no longer be silently dropped by this path. Same-shape finding on the already-fixed platform-toggle surface; UI build + lint green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b6f8eaef0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/api.py Outdated
Comment thread vibe/api.py
@cyhhao

cyhhao commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Round-8 disposition — circuit breaker engaged for the snapshot class:

P1 list-ops whitelist — fixed in d109426 (same-shape on the new verb): _apply_list_ops now only accepts platforms.enabled; any other dotted path raises (test_list_ops_reject_unwhitelisted_paths), so model_hub.sources-style mutations can't bypass ModelHubService.

P1 setup-wizard snapshots — deferred, not patched here. This is the seventh consecutive single-site full-snapshot finding (platforms page → backends page → runtime hook → channels → WeChat QR → credential-apply → wizard). Per the review-loop circuit breaker we stop reviewer-driven whack-a-mole: the systematic sweep is tracked as #1517 (grep every saveConfig caller, classify owned-fields vs snapshot, narrow in one PR with a regression guard). The wizard needs a per-step ownership decision (first-run owns-everything vs reconfigure races) that belongs in that sweep's design, not a rushed patch here.

The wizard thread is intentionally left unresolved pending the owner's merge decision (waive-and-defer-to-#1517 vs fix-here).

@cyhhao

cyhhao commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Setup-wizard P1 fixed in be52217 (owner-directed continuation) — both wizard save paths narrowed to wizard-owned fields:

  • Enablement as operations: Wizard.tsx and Summary.tsx buildConfigPayload now express enablement via the whitelisted list-ops verb (add against the persisted list) instead of a whole platforms.enabled snapshot — a platform concurrently enabled while the wizard sits open (WeChat QR, another surface) is never dropped by a later wizard save. Mid-wizard persists only ever ADD runnable platforms; the finish boundary keeps its stricter existing contract (setup_completed: true validates every enabled platform against the fresh merged state).
  • No stale section round-trips: agents payloads carry only the wizard-owned enable/cli-path/default-agent fields (no ...data.agents.<backend> spreads); runtime sends only default_cwd; gateway/ui/ack_mode/show_duration/language are left to their owners entirely. Summary keeps its genuinely owned update.auto_update toggle and the setup_completed finish signal.

UI build + lint green; backend suites green (179 tests).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be5221717e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/components/Wizard.tsx Outdated
@cyhhao

cyhhao commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in c0b8ce8: sectionFilter now distinguishes editedSections !== undefined — an explicitly passed (possibly empty) list is the step's authoritative edit set, so a Continue from a non-platform step sends NO platform sections; the recorded-section fallback applies only to Finish (no argument). A QR-login credential refresh between wizard steps can no longer be overwritten by an unrelated Continue resubmitting the wizard's stale copy.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0b8ce8a05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/components/Wizard.tsx Outdated
@cyhhao

cyhhao commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in 9f03e0f: the Platform Selection step now diffs its credential draft against a mount-time snapshot (initialDraftRef) and passes ONLY user-dirty sections in the step payload (plus the platforms selection). Sections the user never typed into no longer ride along, so the key-presence filter in persistStep now sees an accurate edit set — a WeChat token acquired by QR login after bootstrap can't be overwritten by the stale empty draft. Untouched-section Continue payloads are selection-only (skip the config save entirely), matching the platformSelectionOnly branch. Baseline updated for the new file's pre-existing any usages (same policy as sibling files).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f03e0f53a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/components/Wizard.tsx Outdated
Comment thread vibe/api.py
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b32948f50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/lib/wizardConfigMutations.ts
Comment thread vibe/api.py Outdated
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ad5e74811

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/lib/wizardConfigMutations.ts Outdated
Comment thread ui/src/components/steps/Summary.tsx Outdated
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fe44ad03b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread core/agent_auth_service.py Outdated
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09c77e2f0b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vibe/ui_server.py Outdated
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fe3dd90f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/lib/wizardConfigMutations.ts
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b388448cf5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ui/src/lib/wizardConfigMutations.ts
@cyhhao

cyhhao commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: fbdf43b3a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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