feat([issue-6208]): emit only changed reviewer fields so task overrides inherit defaults

client/src/components/cos/ReviewerPicker.jsx

@@ -58,11 +58,34 @@ const CUSTOM_MODEL_OPTION = '[custom]';
  * - **Max Iterations** → the numeric `~max=<n>` round cap (blank = slashdo's
  *   built-in default, `0` = loop until clean).
  *
- * Controlled: emits the full next shape via onChange so the parent can store
+ * Controlled: emits the next shape via onChange so the parent can store
  * `reviewers` / `usernames` / `optionalReviewers` / `reviewerModels` /
  * `reviewerEfforts` / `reviewerMaxRounds` / `reviewStopMode` / `reviewerApplies`
  * however it persists them.
  *
+ * `defaults` is the resolved fallback the parent seeded the props from (the
+ * install-wide Code Review Defaults, as token-keyed maps for the pins). When
+ * provided, `emit()` OMITS any key whose value is deep-equal to
+ * `defaults[key]` — a field the user never touched stays absent from the
+ * payload, which is exactly what `resolveReviewerConfig` reads as "inherit".
+ * Without it the picker would freeze the defaults-of-that-moment into a
+ * permanent task override on first touch (#6208). Omit the prop where a full
+ * emit is correct — the surface that edits the defaults themselves has no
+ * fallback to inherit from.
+ *
+ * Absent keeps meaning inherit and an explicitly-empty value keeps meaning
+ * "clear": the comparison is against the resolved default, never against
+ * emptiness, so `{}` / `[]` equal only a matching default and are otherwise
+ * emitted as a real override that clears it. (One key is exempt: the server
+ * drops an empty `reviewers` list before persisting, so `reviewers: []`
+ * resolves to the default chain either way — pre-existing server behavior.)
+ *
+ * Known trade-off: the diff is against the CURRENT default, so an override
+ * that happens to equal it (e.g. set before the default changed to match) is
+ * indistinguishable from "never touched" and reverts to inherit the next time
+ * any other field is edited. The effective reviewers are unchanged at that
+ * moment — the pin only stops shadowing future default changes.
+ *
  * `modelOptions` is the resolved model-picker data, shaped like
  * `useReviewerModelOptions()`'s return: `{ optionsByReviewer, defaultModels,
  * freeText, unavailable, providerDisabled, loaded }`. Callers keep owning their
@@ -101,6 +124,7 @@ export default function ReviewerPicker({
   installed = null,
   stopMode = DEFAULT_REVIEW_STOP_MODE,
   reviewerApplies = false,
+  defaults = null,
   onChange,
   disabled = false,
   showRunFlags = true
@@ -221,17 +245,68 @@ export default function ReviewerPicker({
     ? addable
     : addable.filter(opt => !hiddenAddable.includes(opt));
 
-  const emit = (next) => onChange?.({
-    reviewers: selected,
-    usernames: selectedUsernames,
-    optionalReviewers: optionalTokens,
-    reviewerMaxRounds: maxRoundsMap,
-    reviewerModels: modelsMap,
-    reviewerEfforts: effortsMap,
-    stopMode,
-    reviewerApplies,
-    ...next
-  });
+  // Case-insensitive equality for the token lists (reviewer slugs are already
+  // lowercased; GitHub usernames are case-insensitive). Order matters ONLY for
+  // `reviewers` — the chain runs in click order — so the username lists compare
+  // as sorted sets; otherwise a same-membership reorder would over-emit.
+  const listsEqual = (a, b, ordered) => {
+    if (!Array.isArray(a) || !Array.isArray(b)) return a === b;
+    if (a.length !== b.length) return false;
+    const left = a.map((value) => String(value).toLowerCase());
+    const right = b.map((value) => String(value).toLowerCase());
+    if (!ordered) {
+      left.sort();
+      right.sort();
+    }
+    return left.every((value, index) => value === right[index]);
+  };
+  // Case-insensitive-key equality for the token-keyed pin maps. Values compare
+  // strictly: `0` (loop until clean) must never equal absent, and `{}` equals
+  // only a matching default so an explicit clear is still emitted.
+  const mapsEqual = (a, b) => {
+    const entries = (map) => {
+      if (!map || typeof map !== 'object' || Array.isArray(map)) return map;
+      return Object.entries(map)
+        .map(([key, value]) => [key.toLowerCase(), value])
+        .sort(([x], [y]) => (x < y ? -1 : x > y ? 1 : 0));
+    };
+    const left = entries(a);
+    const right = entries(b);
+    if (!Array.isArray(left) || !Array.isArray(right)) return left === right;
+    if (left.length !== right.length) return false;
+    return left.every(([key, value], index) => right[index][0] === key && right[index][1] === value);
+  };
+  const equalsBaseline = (key, value, baseline) => {
+    if (key === 'reviewers') return listsEqual(value, baseline, true);
+    if (key === 'usernames' || key === 'optionalReviewers') return listsEqual(value, baseline, false);
+    if (key === 'reviewerMaxRounds' || key === 'reviewerModels' || key === 'reviewerEfforts') return mapsEqual(value, baseline);
+    return value === baseline;
+  };
+
+  const emit = (next) => {
+    const full = {
+      reviewers: selected,
+      usernames: selectedUsernames,
+      optionalReviewers: optionalTokens,
+      reviewerMaxRounds: maxRoundsMap,
+      reviewerModels: modelsMap,
+      reviewerEfforts: effortsMap,
+      stopMode,
+      reviewerApplies,
+      ...next
+    };
+    // No baseline (the surface editing the defaults themselves): full snapshot,
+    // exactly as before.
+    if (!defaults || typeof defaults !== 'object') {
+      onChange?.(full);
+      return;
+    }
+    const partial = {};
+    for (const key of Object.keys(full)) {
+      if (!equalsBaseline(key, full[key], defaults[key])) partial[key] = full[key];
+    }
+    onChange?.(partial);
+  };
 
   const toggleOptional = (token) => emit({
     optionalReviewers: isOptional(token) ? withoutToken(token) : [...optionalTokens, token]

client/src/components/cos/ReviewerPicker.test.jsx

@@ -668,4 +668,129 @@ describe('ReviewerPicker', () => {
       expect(details).toHaveAttribute('open');
     });
   });
+
+  describe('defaults-aware emit (#6208)', () => {
+    const DEFAULTS = {
+      reviewers: ['copilot'],
+      usernames: [],
+      optionalReviewers: [],
+      reviewerMaxRounds: {},
+      reviewerModels: {},
+      reviewerEfforts: {},
+      stopMode: 'all',
+      reviewerApplies: false,
+    };
+
+    it('emits the full snapshot when no defaults are provided', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(<ReviewerPicker reviewers={['codex', 'antigravity']} onChange={onChange} />);
+      await user.click(screen.getByLabelText('Remove Codex'));
+      expect(onChange).toHaveBeenCalledWith(expect.objectContaining({
+        reviewers: ['antigravity'],
+        usernames: [],
+        optionalReviewers: [],
+        reviewerMaxRounds: {},
+        reviewerModels: {},
+        reviewerEfforts: {},
+      }));
+    });
+
+    it('omits every key that still equals the defaults when only the stop-mode changes', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex', 'antigravity']}
+          stopMode="all"
+          defaults={{ ...DEFAULTS, reviewers: ['codex', 'antigravity'] }}
+          onChange={onChange}
+        />
+      );
+      await user.selectOptions(screen.getByLabelText('Stop mode:'), 'on-clean');
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ stopMode: 'on-clean' });
+    });
+
+    it('emits only reviewers when one is removed from a seeded list', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex', 'antigravity']}
+          defaults={{ ...DEFAULTS, reviewers: ['codex', 'antigravity'] }}
+          onChange={onChange}
+        />
+      );
+      await user.click(screen.getByLabelText('Remove Codex'));
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ reviewers: ['antigravity'] });
+    });
+
+    it('emits an explicitly-emptied map that clears a default pin (absent ≠ empty)', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex']}
+          optionalReviewers={['codex']}
+          defaults={{ ...DEFAULTS, reviewers: ['codex'], optionalReviewers: ['codex'] }}
+          onChange={onChange}
+        />
+      );
+      await user.click(screen.getByLabelText('Make Codex blocking'));
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ optionalReviewers: [] });
+    });
+
+    it('omits a pin map that already matches the defaults when an unrelated control changes', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex']}
+          reviewerModels={{ codex: 'gpt-5' }}
+          defaults={{ ...DEFAULTS, reviewers: ['codex'], reviewerModels: { codex: 'gpt-5' } }}
+          onChange={onChange}
+        />
+      );
+      await user.click(screen.getByLabelText('Make Codex non-blocking'));
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ optionalReviewers: ['codex'] });
+    });
+
+    it('compares membership lists order-insensitively (reviewers stay order-sensitive)', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex', 'antigravity']}
+          optionalReviewers={['antigravity', 'codex']}
+          defaults={{ ...DEFAULTS, reviewers: ['codex', 'antigravity'], optionalReviewers: ['codex', 'antigravity'] }}
+          onChange={onChange}
+        />
+      );
+      // Reordering the reviewers IS a change (run order); the same-membership
+      // optional set in another order is not — only reviewers is emitted.
+      await user.click(screen.getByLabelText('Move Antigravity earlier'));
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ reviewers: ['antigravity', 'codex'] });
+    });
+
+    it('compares pin-map keys case-insensitively', async () => {
+      const onChange = vi.fn();
+      const user = userEvent.setup();
+      render(
+        <ReviewerPicker
+          reviewers={['codex']}
+          reviewerModels={{ Codex: 'gpt-5' }}
+          defaults={{ ...DEFAULTS, reviewers: ['codex'], reviewerModels: { codex: 'gpt-5' } }}
+          onChange={onChange}
+        />
+      );
+      await user.click(screen.getByLabelText('Make Codex non-blocking'));
+      expect(onChange).toHaveBeenCalledTimes(1);
+      expect(onChange).toHaveBeenCalledWith({ optionalReviewers: ['codex'] });
+    });
+  });
 });

client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx

@@ -675,13 +675,38 @@ export default function GlobalConfigControls({ taskType, config, onUpdate, onTri
               reviewerApplies={config.taskMetadata?.reviewerApplies !== undefined
                 ? (config.taskMetadata?.reviewerApplies === true || config.taskMetadata?.reviewerApplies === 'true')
                 : reviewDefaults.reviewerApplies}
+              // The same fallback the props above were seeded from. The picker
+              // omits whatever still equals it, so touching one control no
+              // longer freezes the rest into a permanent task override (#6208).
+              defaults={{
+                reviewers: reviewDefaults.reviewers,
+                usernames: reviewDefaults.usernames,
+                optionalReviewers: reviewDefaults.optionalReviewers,
+                reviewerMaxRounds: reviewDefaults.reviewerMaxRounds,
+                reviewerModels: seededPins.models,
+                reviewerEfforts: seededPins.efforts,
+                stopMode: reviewDefaults.stopMode,
+                reviewerApplies: reviewDefaults.reviewerApplies,
+              }}
               disabled={updating}
-              onChange={({ reviewers, usernames, optionalReviewers, reviewerMaxRounds, reviewerModels, reviewerEfforts, stopMode, reviewerApplies }) => {
+              onChange={(patch) => {
+                // The picker emits only what differs from `defaults` above, so
+                // rebuild the reviewer slice from scratch: strip every override
+                // key (a key that reverted to the default must be DELETED, not
+                // left pinning its old value), then re-apply what arrived.
                 // Drop the legacy single `reviewer` key so storage converges on `reviewers`.
                 const { reviewer: _reviewer, ...rest } = config.taskMetadata || {};
-                onUpdate(taskType, {
-                  taskMetadata: { ...rest, reviewers, usernames, optionalReviewers, reviewerMaxRounds, reviewerModels, reviewerEfforts, reviewStopMode: stopMode, reviewerApplies }
-                });
+                for (const key of REVIEW_CONFIG_KEYS) delete rest[key];
+                const taskMetadata = { ...rest };
+                if (patch.reviewers !== undefined) taskMetadata.reviewers = patch.reviewers;
+                if (patch.usernames !== undefined) taskMetadata.usernames = patch.usernames;
+                if (patch.optionalReviewers !== undefined) taskMetadata.optionalReviewers = patch.optionalReviewers;
+                if (patch.reviewerMaxRounds !== undefined) taskMetadata.reviewerMaxRounds = patch.reviewerMaxRounds;
+                if (patch.reviewerModels !== undefined) taskMetadata.reviewerModels = patch.reviewerModels;
+                if (patch.reviewerEfforts !== undefined) taskMetadata.reviewerEfforts = patch.reviewerEfforts;
+                if (patch.stopMode !== undefined) taskMetadata.reviewStopMode = patch.stopMode;
+                if (patch.reviewerApplies !== undefined) taskMetadata.reviewerApplies = patch.reviewerApplies;
+                onUpdate(taskType, { taskMetadata });
               }}
             />
           </div>