test: direct suite for settings-hub shared merge/persist helpers

[ndycode]

Summary

• Adds test/settings-hub-shared.test.ts (14 cases) for the merge/persist helpers in lib/codex-manager/settings-hub/shared.ts. The module ships ...ForTests injection variants that nothing exercised; the settings panels were the only callers.

What Changed

One new test file, mocking only the disk seams (loadDashboardDisplaySettings/saveDashboardDisplaySettings/savePluginConfig via importOriginal spreads) while the merge logic, the real queued-retry policy, and the real backendSettingsEqual/cloneDashboardSettings stay live:

• copyDashboardSettingValue: array values are cloned, never shared — a later mutation of the target must not write through to the source settings object.
• applyDashboardDefaultsForKeys / mergeDashboardSettingsForKeys: only the listed keys move; inputs are never mutated.
• persistDashboardSettingsSelection — the load-merge-save transaction the panels rely on:
  • re-reads the latest settings inside the queue slot, so a concurrent edit to an unrelated key on disk is preserved instead of clobbered by the panel's stale view (the core reason this helper exists);
  • retries a transient EBUSY through the real queued-retry policy with no user-facing warning;
  • on persistent failure, warns with the exact Settings save failed (<scope>) after retries: <message> line and returns the clone-normalized selection as fallback (never the caller's object).
• persistBackendConfigSelection: saves the backend patch, returns a defensive clone (unsupportedCodexFallbackChain not reference-shared), and the warn + fallback path.
• clampBackendNumber: min/max clamping and rounding sweep.

Validation

• npm run typecheck (also runs in the pre-commit hook)
• npx eslint test/settings-hub-shared.test.ts --max-warnings=0
• npm test -- test/settings-hub-shared.test.ts — 14/14

Docs and Governance Checklist

• Test-only; no user-visible behavior, commands, settings, or paths changed

Risk and Rollback

• Risk level: minimal — one new test file, no source changes.
• Rollback plan: delete the test file.

Additional Notes

• Complements test: cover dashboard settings clone defaults and equality semantics #569 (settings data clone/equality) and test: direct suite for the settings preview builders #583 (preview rendering); this one owns the merge/persist layer between them. Independent of every other open branch; based on main.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB

---

Generated by Claude Code

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

adds test/settings-hub-shared.test.ts with 14 cases covering the merge/persist helpers in lib/codex-manager/settings-hub/shared.ts. no source changes.

• clone isolation: verifies array values in copyDashboardSettingValue are never reference-shared between source and target.
• concurrent re-read: confirms persistDashboardSettingsSelection re-reads from disk inside the queue slot so a concurrent write to an unrelated key is preserved.
• ebusy exhaustion: both dashboard and backend paths now have four-consecutive-EBUSY tests that assert all four attempts fire and the warn+fallback path triggers; this closes the gap flagged in the previous review round.

Confidence Score: 5/5

test-only change with no source modifications; safe to merge.

one new test file, no production code touched. the ebusy exhaustion and fallback paths are correctly exercised; the concurrency re-read case is properly constructed. the only thing worth a follow-up is the ~750ms of real sleep in three tests, which is a run-time comfort issue, not a correctness problem.

no files require special attention.

Important Files Changed

Filename	Overview
test/settings-hub-shared.test.ts	14-case suite for settings-hub shared helpers; covers clone isolation, key-scoped merge, concurrent re-read, EBUSY exhaustion on both paths, and clampBackendNumber. Real sleep is used for retry tests (~750ms total), and one minor coverage gap around loadDashboardDisplaySettings call-count assertions.

Sequence Diagram

sequenceDiagram
    participant Test
    participant persistDashboardSettingsSelectionForTests
    participant withQueuedRetry
    participant loadDashboardDisplaySettings (mock)
    participant saveDashboardDisplaySettings (mock)

    Test->>persistDashboardSettingsSelectionForTests: selected, keys, scope
    persistDashboardSettingsSelectionForTests->>withQueuedRetry: "pathKey, task(), { sleep }"

    loop up to 4 attempts (EBUSY path)
        withQueuedRetry->>loadDashboardDisplaySettings (mock): re-read latest
        loadDashboardDisplaySettings (mock)-->>withQueuedRetry: latest settings
        withQueuedRetry->>saveDashboardDisplaySettings (mock): merged settings
        saveDashboardDisplaySettings (mock)-->>withQueuedRetry: EBUSY (or success)
        withQueuedRetry->>withQueuedRetry: sleep(backoff) if retryable
    end

    alt success
        withQueuedRetry-->>persistDashboardSettingsSelectionForTests: merged
        persistDashboardSettingsSelectionForTests-->>Test: merged
    else exhausted / non-retryable
        withQueuedRetry-->>persistDashboardSettingsSelectionForTests: throw error
        persistDashboardSettingsSelectionForTests->>persistDashboardSettingsSelectionForTests: warnPersistFailure
        persistDashboardSettingsSelectionForTests-->>Test: cloneDashboardSettings(selected)
    end

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
test/settings-hub-shared.test.ts:20-23
the three retry/exhaustion tests run against real `sleep` (imported live from `utils.js` inside `shared.ts`). that's 50 ms for the single-retry case, 350 ms each (50+100+200) for the two ebusy-exhaustion cases — roughly 750 ms of real wall-clock time added to the suite. mocking `../../utils.js` to stub `sleep` as `() => Promise.resolve()` makes them instant and deterministic without needing fake-timer boilerplate, which is the pattern `stream-failover.test.ts` already uses for the same reason.

```suggestion
vi.mock("../lib/config.js", async (importOriginal) => ({
	...(await importOriginal<typeof import("../lib/config.js")>()),
	savePluginConfig: savePluginConfigMock,
}));

vi.mock("../lib/utils.js", async (importOriginal) => ({
	...(await importOriginal<typeof import("../lib/utils.js")>()),
	sleep: () => Promise.resolve(),
}));
```

_{Reviews (2): Last reviewed commit: "test: cover EBUSY retry exhaustion for b..." | Re-trigger Greptile}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Warning

Review limit reached

@ndycode, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 4 minutes and 53 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 82a5c4bf-58ed-456d-bc93-9302c18b8d3b

📥 Commits

Reviewing files that changed from the base of the PR and between b566656 and 8457e39.

📒 Files selected for processing (1)

• test/settings-hub-shared.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/audit-65-settings-hub-shared-tests

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/audit-65-settings-hub-shared-tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}