Skip to content

Make provider profile activation compensate cross-store failures safely #2315

Open

Description

@coderabbitai

Summary

Add cross-store compensation for the activation batch in ClineProvider.persistProviderProfile. Add concurrent-view failure tests. Deliver this work as a separate change from PR #2237.

Requested by @easonLiangWorldedtech.

Current behavior and rationale

persistProviderProfile saves the profile through ProviderSettingsManager.saveConfig before activation. Its activation batch uses Promise.allSettled for:

  • Profile-list metadata in ContextProxy.
  • setValue("currentApiConfigName", name), including durable per-view selection persistence.
  • ProviderSettingsManager.setModeConfig(mode, id).
  • ContextProxy.setProviderSettings(providerSettings).
  • The view-local apiConfiguration overlay.

If a member rejects, the function throws the first rejection after all members settle. Successful members remain committed. The throw skips updateTaskApiHandlerIfNeeded and persistStickyProviderProfileToCurrentTask. The stored profile, selected profile, shared settings, view overlay, and running task can therefore disagree.

The batch's partial-commit semantics predate PR #2237. That PR routes the selected-profile write through setValue and adds the view overlay update. With only apiConfiguration, _saveViewLocalStateFromMutation does not write durable viewStates. The added durable-write failure surface comes through setValue("currentApiConfigName", name).

Required changes

  • Define the previous activation state across ProviderSettingsManager secret-backed storage, mode mappings, ContextProxy, durable per-view selections, and view-local overlays.
  • Choose a single compensation owner: persistProviderProfile, or an explicit caller contract applied consistently.
  • Capture the previous state before writes and compensate committed changes after an activation failure. Include an explicit policy for the preceding saveConfig write, covering both existing and newly created profiles.
  • Cover the currently unguarded ContextProxy.setProviderSettings and ProviderSettingsManager.setModeConfig writes.
  • Preserve newer writes from another view during compensation. Do not restore a stale whole-store snapshot.
  • Build on _restoreSharedValuesAfterFailedViewWrite and ContextProxy.getStateWriteGeneration where applicable. These guards cover ContextProxy keys only; define equivalent ownership or serialization for the other state.
  • Reconcile updateProfileModel, which attempts profile restoration, with upsertProviderProfile, which currently logs errors, shows an error message, and returns undefined.
  • Define error reporting and recovery when compensation itself fails. Preserve the original activation failure and expose any incomplete recovery without logging credentials.

Affected areas and anchors

Line numbers are anchors supplied in the originating discussion and can move:

  • src/core/webview/ClineProvider.ts: persistProviderProfile, supplied Lines 2916–2941.
  • src/core/webview/ClineProvider.ts: _restoreSharedValuesAfterFailedViewWrite, supplied Lines 6396–6428.
  • src/core/webview/ClineProvider.ts: upsertProviderProfile, updateProfileModel, setValue, _saveViewLocalStateFromMutation, updateTaskApiHandlerIfNeeded, and persistStickyProviderProfileToCurrentTask.
  • src/core/config/ContextProxy.ts: provider-settings writes and per-key write generations.
  • src/core/config/ProviderSettingsManager.ts: profile persistence and mode mappings.
  • src/core/webview/__tests__/ClineProvider.spec.ts: focused activation and concurrent-view regression tests.
  • Relevant storage-manager tests where compensation requires new contracts.

Acceptance criteria

  • Inject a failure in each durable activation write after other writes have succeeded. Verify that compensation settles before the mutation finishes.
  • Verify consistency of profile storage, profile-list metadata, shared selection, shared provider settings, mode mapping, durable per-view selection, and view-local overlay after failure.
  • Verify that failed activation does not leave the task handler or sticky task profile inconsistent with the recovered state.
  • Test existing-profile updates and newly created profiles.
  • Test a second live view saving or activating a profile while the first view compensates. Newer valid writes must survive, including same-value writes with a newer generation.
  • Test delayed writes and different completion orders. Compensation must not race an unsettled activation write.
  • Test compensation failure and subsequent retry. Incomplete recovery must be reported explicitly.
  • Verify consistent compensation behavior for upsertProviderProfile and updateProfileModel, without duplicate or conflicting restoration.
  • Retain successful activation behavior and sibling-view synchronization.
  • Keep provider credentials out of diagnostics and failure messages.

Source context

This issue records deferred work. Creating it does not establish that the review finding is fixed or decide the PR's merge status.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions