fix(flags): skip flag revalidation when evaluation is disabled or pending

[mvanhorn]

Description

isEnabled() and getValue() now require evaluation to be active before revalidating a stale entry or fetching a cache miss. Cached results and overrides are still returned immediately, and a miss still returns the existing loading or fallback value. Active stale reads still revalidate. Server flags tests cover disabled and pending stale reads, inactive cache misses, overrides while disabled, and an active stale read, and a patch changeset records the skip.

After a flag is cached, synchronous isEnabled() and getValue() reads still start a background evaluation once that entry is stale, including when the manager is disabled or the session is still pending. The caller keeps receiving the cached flag while an extra evaluation request goes out. On a stale cache hit, both methods call revalidate() when the entry is stale and shouldSkipFetch() is false. On a cache miss, they start a background getFlag() when canFetchOnRead() is true. Neither branch checks config.disabled or config.isPending.

Closes #1091

Slice

• Issue: #
• Scope / owning surface:
• Dependencies or overlapping PRs: None

Checklist

• This branch started from current staging and does not include another unmerged PR unless it is named above.
  Not claimed: npm test could not run in this environment (dotenv is not installed).
• This is one independently reviewable slice; unrelated cleanup or refactors are in separate PRs.
• I checked open PRs for overlapping files, contracts, schemas, or deployment configuration and made any dependency explicit above.
• This PR targets staging; after it closes, this branch will not be reused for another change.
  Not claimed: staging was not run locally either.
• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
  Not claimed: the same reason as the box above applies.

AI disclosure

AI was used for assistance.

• Tools: Cursor.

---

Summary by cubic

isEnabled() and getValue() skip background flag revalidation and cache-miss fetches when evaluation is disabled or the session is pending, instead of firing unnecessary network requests.

• Stale cache hits and cache misses now require config.disabled and config.isPending to be false before revalidating or fetching.
• Cached results and overrides still return immediately, and cache misses still return the existing loading or fallback value.
• Active stale reads still revalidate as before.
• Adds server tests covering disabled and pending stale reads as separate tests sharing a helper, inactive cache misses, overrides while disabled, and active stale reads for each of isEnabled() and getValue() on separate managers.

Fixes #1091.

^{Written for commit 23907c0. Summary will update on new commits.}

[vercel]

@mvanhorn is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: f50f1a54-0f0b-4c19-9837-d746705fc621

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adjusts when flag evaluation triggers background refreshes.

The read behavior looks correct, but the test must satisfy the repository's rule against await inside loops before merging.

Findings

1.  `getValue()` refresh is untested ▶
2.  Test uses forbidden loop awaits ▶

Summary

isEnabled() and getValue() now skip background work while evaluation is disabled or pending.

• Flag reads stop fetching in the background when evaluation is inactive.

_{Reviews (1) · Last reviewed commit: "fix(flags): skip flag revalidation when ..." · Reviewed by Greptile}

[greptile-apps]

getValue() refresh is untested

The active stale-read test calls isEnabled() before getValue() on the same entry. isEnabled() starts the refresh and marks the entry as refreshing, so getValue() cannot start another one. The fetch-count assertion would still pass if getValue() stopped refreshing active stale entries. Test each method with a separate manager or flag.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[mvanhorn]

Split in 23907c0: getValue and isEnabled each get their own fresh manager now, and the getValue test checks the stale key shows up in the revalidation request. Removing the revalidate call from getValue makes only that test fail.

[greptile-apps]

Test uses forbidden loop awaits

The new inactive-read test uses await inside a for...of loop. The repository guide explicitly says “Don't use await inside loops.” Define separate tests for the disabled and pending cases, with a shared helper if needed. This repository requirement must be satisfied before merging.

Context Used: Ultracite Rules - AI-Ready Formatter and Linter (source)

[mvanhorn]

Fixed in 23907c0: the loop is gone, the disabled and pending cases are separate tests sharing a helper, same assertions.