Skip to content

test: direct suite for the settings preview builders#583

Merged
ndycode merged 2 commits into
mainfrom
claude/audit-64-settings-preview-tests
Jun 11, 2026
Merged

test: direct suite for the settings preview builders#583
ndycode merged 2 commits into
mainfrom
claude/audit-64-settings-preview-tests

Conversation

@ndycode

@ndycode ndycode commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds test/settings-preview.test.ts (12 cases) for lib/codex-manager/settings-preview.ts — the preview-first settings UX renderer, previously reachable only through the interactive settings-hub panels.

What Changed

One new test file pinning, with the real resolveMenuLayoutMode from settings-hub/shared.ts (no mocks):

  • normalizeStatuslineFields: undefined/empty fall back to the documented defaults (and the empty-input path returns a defensive copy, not the shared default array); duplicates collapse to first-occurrence order.
  • buildSummaryPreviewText: default composition (last-used + limits with cooldowns; status text only when status badges are hidden — the inverse dependency the preview exists to explain); the cooldown toggle; part ordering following menuStatuslineFields; and both nothing-visible explanations (the status-badge-dependency note vs the generic message when no status field is configured).
  • buildAccountListPreview: the demo row label with both badges, individual badge toggles, and the layout text driven by the real resolver — explicit expanded-rows mode and the legacy menuShowDetailsForUnselectedRows boolean both produce "details shown on all rows".
  • highlightPreviewToken: returns plain text on non-TTY stdout; on a forced TTY it ANSI-wraps the token (stripping escapes recovers the exact plain text), and in the composed summary only the focused part carries escapes — the trailing limits segment stays clean.

TTY forcing uses Object.defineProperty on process.stdout.isTTY with the original descriptor restored in finally.

Validation

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

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

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-preview.test.ts — a 12-case direct suite for the settings-preview builders in lib/codex-manager/settings-preview.ts. no source changes; the three issues flagged in the previous round (sequential-execution comment, writable: true on the defineProperty call, and the fromUndefined reference-identity assertion) are all present in this version.

  • normalizeStatuslineFields (2 cases): undefined/empty fall back to defaults returning a defensive copy; duplicates deduplicate to first-occurrence order.
  • buildSummaryPreviewText (4 cases): default composition, cooldown toggle, field ordering, and both nothing-visible messages (status-badge-dependency note vs. generic message).
  • buildAccountListPreview (3 cases): demo row with/without badges, expanded-rows layout via both the explicit menuLayoutMode field and the legacy menuShowDetailsForUnselectedRows boolean.
  • highlightPreviewToken (3 cases): non-TTY plain text, TTY ANSI wrapping with strip-and-recover assertion, focused-part isolation in the composed summary.

Confidence Score: 5/5

test-only pr with no source changes; all previous review issues have been addressed in the submitted version.

the change is a single new test file that adds 12 assertions against an existing renderer module. the setStdoutTty helper correctly handles nested calls and restores the original descriptor in finally. the fromUndefined reference-identity guard is present. the sequential-execution comment is in place. no production code is touched.

no files require special attention.

Important Files Changed

Filename Overview
test/settings-preview.test.ts new 12-case suite covering normalizeStatuslineFields, buildSummaryPreviewText, buildAccountListPreview, and highlightPreviewToken; prior review issues (concurrency note, writable:true, fromUndefined reference check) are all addressed

Reviews (2): Last reviewed commit: "test: harden TTY helper and pin undefine..." | Re-trigger Greptile

lib/codex-manager/settings-preview.ts renders the preview-first
settings UX but was reachable only through the settings-hub panels.
These 12 cases pin: statusline-field normalization (defaults,
first-occurrence dedup, defensive copy), summary composition (default
parts, cooldown toggle, field ordering, the two nothing-visible
explanations incl. the status-badge dependency), the account-list row
(badge toggles, layout text from the real resolveMenuLayoutMode incl.
the legacy boolean), and highlightPreviewToken TTY gating with
ANSI-stripped equality against the plain render.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
@chatgpt-codex-connector

Copy link
Copy Markdown

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

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

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 12 minutes and 47 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: fee911e3-9b6d-42cd-88cb-c51cf15385e6

📥 Commits

Reviewing files that changed from the base of the PR and between b566656 and 52e4245.

📒 Files selected for processing (1)
  • test/settings-preview.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-64-settings-preview-tests
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/audit-64-settings-preview-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

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

Comment thread test/settings-preview.test.ts
Comment thread test/settings-preview.test.ts
Comment thread test/settings-preview.test.ts
Review follow-ups: setStdoutTty now sets writable: true (matching how
tty streams expose isTTY) and documents that the TTY-sensitive tests
must stay sequential; the undefined input path is now also guarded
against returning the shared DEFAULT_STATUSLINE_FIELDS reference.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
@ndycode ndycode merged commit c23e518 into main Jun 11, 2026
2 checks passed
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.

2 participants