feat: add complete Ruby Server SDK - #1
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a Ruby FeatBit Server SDK with local evaluation, WebSocket synchronization, event delivery, lifecycle APIs, Console and Rails examples, documentation, tests, CI, static analysis, packaging, and resource-audit scripts. ChangesRuby Server SDK
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WebSocketDataSynchronizer
participant InMemoryDataStore
participant Evaluator
participant EventProcessor
Client->>WebSocketDataSynchronizer: start synchronization
WebSocketDataSynchronizer->>InMemoryDataStore: apply full or patch data
Client->>Evaluator: evaluate flag for user
Evaluator->>InMemoryDataStore: read flag and segment data
Client->>EventProcessor: enqueue evaluation or tracking event
Client->>WebSocketDataSynchronizer: close
Client->>EventProcessor: flush and close
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (12)
spec/event_processor_spec.rb (2)
121-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider asserting the sender was not invoked spuriously.
The idempotency assertion is good; adding
expect(processor.close).to be(true)after the threads join would also lock in that a late, serialized call returns the cached result rather than re-running shutdown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/event_processor_spec.rb` around lines 121 - 127, Extend the “makes concurrent close idempotent” example after the thread results assertion to call processor.close again and assert it returns true, verifying the cached shutdown result is reused for a later serialized call without re-invoking shutdown.
18-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWhite-box
allocate+ ivar injection couples the spec toinitializeinternals.Every ivar added to the constructor must be mirrored here or the test breaks (or worse, silently exercises a half-built object). You can get the same behaviour from a real instance by holding the worker inside a blocking sender until the queue is saturated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/event_processor_spec.rb` around lines 18 - 33, Rewrite the “drops instead of blocking when the bounded queue is full” spec to use a normally initialized EventProcessor instance rather than described_class.allocate and manual instance-variable setup. Hold the worker in a blocking sender or equivalent controlled state until its bounded queue is saturated, then verify enqueue returns false and dropped_events equals one while preserving the existing capacity and non-blocking behavior assertions.spec/web_socket_data_synchronizer_spec.rb (1)
28-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest name promises ordering coverage that the body does not exercise.
Only one flag is patched, so the
sort_by { item_version }path is never actually validated. Add a second patch item for the same key with an olderupdatedAtdelivered first, and assert the newer value wins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/web_socket_data_synchronizer_spec.rb` around lines 28 - 48, Update the test around synchronizer.process_message to include two patch entries for the same flag: deliver the older updatedAt and value first, followed by the newer updatedAt and value, then assert the final stored flag uses the newer value while changes still reports only "welcome".lib/featbit/event_processor.rb (2)
123-136: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffWorker busy-polls two queues at 10 ms intervals.
pop_with_timeoutnever blocks; the worker wakes ~100 times/second for the lifetime of the process even when completely idle. Pushing control messages onto the sameSizedQueue(or using aConditionVariable) would let the worker block on a real timeout instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/event_processor.rb` around lines 123 - 136, Update pop_with_timeout so the worker blocks while both queues are empty instead of repeatedly sleeping for 10 ms; consolidate wake-up signaling through the existing queue mechanism or a ConditionVariable, while preserving priority for `@control_queue`, timeout expiry, and returning nil when no item arrives.
157-176: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
DeliveryRejectedbypasses retries for transient 5xx responses.
post_batchraisesDeliveryRejectedfor every non-2xx response, and the innerrescue DeliveryRejected; raiseskips the retry loop. That is right for 4xx, but 502/503/504 and throttling (429) are transient and worth retrying with backoff. Consider distinguishing retryable status codes, and adding a sleep between attempts — the currentretryhammers the server three times immediately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/event_processor.rb` around lines 157 - 176, The send_slice retry flow must retry transient delivery failures while preserving immediate propagation for non-retryable rejections. Update the DeliveryRejected handling to distinguish retryable HTTP statuses such as 429, 502, 503, and 504, retry only those up to the existing attempt limit, and add backoff sleep between attempts instead of immediate retry; keep final logging and false-return behavior unchanged.lib/featbit/data_store.rb (4)
53-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent failure on sync errors makes stale-data issues hard to diagnose.
init/upsertswallow anyStandardErrorand just returnfalsewith no logging, unlikeEvaluator, which accepts alogger:and reports failures. Ifnormalize_flag/normalize_hashthrows on an unexpected payload shape, the synchronizer'schanged =@data_store.init(data)(see the synchronizer'sprocess_message, context snippet 1) simply treats it as "no change" with zero diagnostic trail — directly relevant to the "stale entity versions" reliability goal called out in the PR objectives. Consider accepting an optionallogger:ininitializeand logging inside the rescue blocks.Also applies to: 81-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/data_store.rb` around lines 53 - 79, Update DataStore initialization and the init/upsert error paths to accept an optional logger, matching Evaluator’s logger pattern, and use it to report rescued StandardError details before returning false. Preserve the existing false-return behavior and ensure both init and upsert provide diagnostic context for normalization or payload failures.
44-51: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
all_flagsdeep-clones values even for key-only callers.Every flag payload (rules, variations, targets) is deep-cloned here, but the primary caller only needs the key set: the full-sync WebSocket path snapshots current flags via
old_keys =@data_store.all_flags.keys`` and again forchanged_keys, discarding the values immediately. Consider exposing a lightweight `flag_keys` accessor (`@mutex.synchronize { `@flags.keys` }`) for key-only use to avoid the deep-dup cost on this hot full-sync path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/data_store.rb` around lines 44 - 51, The full-sync WebSocket path unnecessarily deep-clones flag payloads through all_flags when it only needs keys. Add a lightweight flag_keys accessor in the data store that returns `@flags.keys` under `@mutex` synchronization, then update the old_keys and changed_keys snapshots to use flag_keys instead of all_flags.keys while preserving archived-flag filtering behavior as required.
171-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
Marshalfor deep-cloning.
deep_dupis applied only to already-normalized, plain Hash/Array/String/Numeric/Boolean data that this same process produced — it isn't deserializing an external Marshal stream, so the RCE label from the scanners is a false positive here. That said, the round-trip throughMarshal.dump/Marshal.loadis slower than a manual recursive clone and will keep tripping Brakeman/OpenGrep/ast-grep in CI (this PR explicitly validates RuboCop/CI checks per the objectives).♻️ Manual recursive clone
- def deep_dup(value) - Marshal.load(Marshal.dump(value)) - end + def deep_dup(value) + case value + when Hash + value.each_with_object({}) { |(k, v), h| h[deep_dup(k)] = deep_dup(v) } + when Array + value.map { |v| deep_dup(v) } + else + value + end + end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/data_store.rb` around lines 171 - 173, Replace the Marshal-based implementation in deep_dup with a manual recursive clone for the normalized Hash/Array/String/Numeric/Boolean values it receives. Recursively duplicate Hash keys and values and Array elements, duplicate mutable Strings, and return immutable scalar values unchanged; preserve deep_dup’s existing behavior without introducing Marshal calls.Source: Linters/SAST tools
81-114: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTwo-phase mutex for fallback version is not atomic.
incoming_version =@mutex.synchronize{@Version+ 1 } if incoming_version.to_i <= 0(line 87) reads@versionin its own critical section, separate from the write's critical section (lines 88-110). Two concurrent upserts for the same key that both lacktimestamp/updatedAtcan compute the identical synthetic version; the second call's write is then rejected by the<=check, silently dropping a distinct update. This only affects items without an explicit version/timestamp, so impact is limited, but computing the fallback inside the same synchronized block would remove the race.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/data_store.rb` around lines 81 - 114, The fallback version calculation in upsert is performed under a separate mutex scope from the state update, allowing concurrent unversioned upserts to receive the same version. Move the fallback `@version` increment/calculation into the existing synchronized block in upsert, ensuring each missing-version update gets a unique version while preserving explicit-version handling and rejection logic.spec/data_store_spec.rb (1)
5-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for archived-item filtering and segment upserts.
Current tests only exercise the flags path (
flag,all_flags,upsert(:flags, ...)) and version-based patch acceptance.data_store.rb'sisArchivedfiltering (inflag/segment/all_flags) and the:segmentsbranch ofupsert(with its own@segment_versionsstaleness check) are untested here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/data_store_spec.rb` around lines 5 - 48, Add focused examples to FeatBit::InMemoryDataStore coverage for archived filtering through flag, segment, and all_flags, verifying archived items are excluded while active items remain available. Also exercise upsert(:segments, ...) with newer, stale, and equal versions as applicable, asserting the segment data and `@segment_versions` staleness behavior matches the flags path.lib/featbit/options.rb (1)
17-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate default literals between constructor and rescue fallback.
The rescue block (lines 46-58) re-hardcodes the same default values already declared as keyword defaults (lines 18-22). If a default changes in one place but not the other, the error-fallback path silently diverges from the intended configuration.
♻️ Suggested refactor: extract shared default constants
module FeatBit class Options DEFAULT_STREAMING_URL = "wss://app-eval.featbit.co" DEFAULT_EVENT_URL = "https://app-eval.featbit.co" + DEFAULT_START_WAIT = 5.0 + DEFAULT_EVENTS_CAPACITY = 10_000 + DEFAULT_EVENTS_FLUSH_INTERVAL = 1.0 + DEFAULT_EVENTS_BATCH_SIZE = 50 + DEFAULT_CONNECT_TIMEOUT = 5.0 + DEFAULT_READ_TIMEOUT = 10.0 + DEFAULT_RECONNECT_DELAY = 1.0 - def initialize(env_secret: "", streaming_url: DEFAULT_STREAMING_URL, - event_url: DEFAULT_EVENT_URL, start_wait: 5.0, offline: false, + def initialize(env_secret: "", streaming_url: DEFAULT_STREAMING_URL, + event_url: DEFAULT_EVENT_URL, start_wait: DEFAULT_START_WAIT, offline: false, ...) ... rescue StandardError => e warn("FeatBit options error: #{e.message}") `@env_secret` = "" `@streaming_url` = DEFAULT_STREAMING_URL `@event_url` = DEFAULT_EVENT_URL - `@start_wait` = 5.0 + `@start_wait` = DEFAULT_START_WAIT ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/featbit/options.rb` around lines 17 - 60, Update Options#initialize to centralize the shared fallback values in default constants or a single reusable defaults structure, then use those references for both the keyword defaults and rescue fallback assignments. Remove the duplicated literals while preserving the existing safe fallback behavior, including offline and disabled events.spec/client_spec.rb (1)
62-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest doesn't actually exercise the "broken store" path it targets.
Object.newnormalizes to an invalid, empty-keyUser, soEvaluator#evaluateshort-circuits onuser&.valid?before ever callingbroken_store.flag/segment. The store's raising behavior is never actually exercised; use a valid user (e.g.{ key: "u1" }) to genuinely cover the data-store-raises scenario.♻️ Suggested fix
- expect { client.variation("x", Object.new, "fallback") }.not_to raise_error - expect(client.variation("x", Object.new, "fallback")).to eq("fallback") + expect { client.variation("x", { key: "u1" }, "fallback") }.not_to raise_error + expect(client.variation("x", { key: "u1" }, "fallback")).to eq("fallback")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/client_spec.rb` around lines 62 - 71, Update the “never raises from public evaluation methods” example to pass a valid user, such as one with key “u1”, to both client.variation calls so Evaluator#evaluate reaches broken_store.flag or segment. Preserve the existing assertions that evaluation does not raise and returns the fallback value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 16: Update the actions/checkout step in the CI workflow to disable
persisted credentials and restrict the workflow token to read-only permissions.
Configure checkout with persist-credentials disabled and set the job or workflow
permissions to only the minimum required read access, preserving the existing
checkout behavior.
In `@lib/featbit/evaluator.rb`:
- Around line 138-139: Update the MatchRegex and NotMatchRegex branches in the
evaluator to enforce a process-level Regexp.timeout before evaluating targeting
patterns, while preserving their existing nil and match semantics. Avoid
unnecessary repeated Regexp.new compilation in this hot path by reusing a
bounded compiled pattern if an appropriate cache already exists.
In `@lib/featbit/event_processor.rb`:
- Around line 151-155: Update send_batch so it attempts send_slice for every
slice produced by batch.each_slice, rather than using short-circuiting all?.
Accumulate whether all slices succeeded while continuing through the remaining
slices, preserving the existing true result for empty batches and returning the
aggregate success status.
- Around line 63-77: Update close to short-circuit when `@thread` is nil or no
longer alive, returning a successful idempotent result without enqueueing STOP
or waiting on the control queue. Ensure `@close_result` is assigned on this path
so subsequent close calls return immediately, while preserving the existing
shutdown flow for a live worker.
In `@lib/featbit/user.rb`:
- Around line 7-17: Update User#initialize to defensively copy and deeply freeze
both key and name using the existing deep_copy and deep_freeze helpers, matching
the treatment of custom. Preserve the existing nil behavior for name and ensure
the resulting User cannot reflect mutations to caller-owned string objects.
- Around line 23-31: Update the [] method’s custom-attribute lookup to check
whether the requested key exists rather than using truthiness, so stored false
values are returned as false. Preserve the existing
original-key-then-normalized-key lookup order and nil result for missing keys.
In `@lib/featbit/web_socket_data_synchronizer.rb`:
- Around line 192-206: The process_patch method incorrectly reports a partially
applied patch as unchanged because it returns results.all?. Replace the
aggregate status with an “any item applied” result, while preserving
changed_keys collection and deduplication so successfully upserted flags or
segments notify listeners and allow the synchronizer to reach READY even when
other items are stale.
- Around line 110-117: Update the ensure block in the connection loop to call
safe_close_socket(socket) unconditionally before clearing `@socket`, so sockets
are closed whenever an iteration exits, including configure_socket or send
failures; preserve the existing mutex-based `@socket` reset.
In `@README.md`:
- Around line 72-73: Update the README documentation links for the Console and
Rails examples to use repository-relative paths prefixed with ./examples/
instead of root-relative paths. Apply the same relative-link convention to the
review guide link referenced elsewhere in the README, using ./docs/ so all links
remain within the repository on GitHub.
In `@scripts/live_integration_check.rb`:
- Around line 27-45: Validate that the successful result from variation_detail
is a boolean before starting concurrent evaluations. Update the initial flag
evaluation around detail and detail.success? to reject non-boolean values, while
preserving the existing error handling and thread checks.
In `@spec/evaluator_spec.rb`:
- Around line 5-9: Add a percentage-rollout example in the FeatBit::Evaluator
specs using a partial range such as [0, 0.5], with a fixed user key and an
assertion for its expected variation. Ensure the case exercises the MD5-based
bucketing path in in_bucket? and preserves the existing evaluator setup and
full-range tests.
---
Nitpick comments:
In `@lib/featbit/data_store.rb`:
- Around line 53-79: Update DataStore initialization and the init/upsert error
paths to accept an optional logger, matching Evaluator’s logger pattern, and use
it to report rescued StandardError details before returning false. Preserve the
existing false-return behavior and ensure both init and upsert provide
diagnostic context for normalization or payload failures.
- Around line 44-51: The full-sync WebSocket path unnecessarily deep-clones flag
payloads through all_flags when it only needs keys. Add a lightweight flag_keys
accessor in the data store that returns `@flags.keys` under `@mutex`
synchronization, then update the old_keys and changed_keys snapshots to use
flag_keys instead of all_flags.keys while preserving archived-flag filtering
behavior as required.
- Around line 171-173: Replace the Marshal-based implementation in deep_dup with
a manual recursive clone for the normalized Hash/Array/String/Numeric/Boolean
values it receives. Recursively duplicate Hash keys and values and Array
elements, duplicate mutable Strings, and return immutable scalar values
unchanged; preserve deep_dup’s existing behavior without introducing Marshal
calls.
- Around line 81-114: The fallback version calculation in upsert is performed
under a separate mutex scope from the state update, allowing concurrent
unversioned upserts to receive the same version. Move the fallback `@version`
increment/calculation into the existing synchronized block in upsert, ensuring
each missing-version update gets a unique version while preserving
explicit-version handling and rejection logic.
In `@lib/featbit/event_processor.rb`:
- Around line 123-136: Update pop_with_timeout so the worker blocks while both
queues are empty instead of repeatedly sleeping for 10 ms; consolidate wake-up
signaling through the existing queue mechanism or a ConditionVariable, while
preserving priority for `@control_queue`, timeout expiry, and returning nil when
no item arrives.
- Around line 157-176: The send_slice retry flow must retry transient delivery
failures while preserving immediate propagation for non-retryable rejections.
Update the DeliveryRejected handling to distinguish retryable HTTP statuses such
as 429, 502, 503, and 504, retry only those up to the existing attempt limit,
and add backoff sleep between attempts instead of immediate retry; keep final
logging and false-return behavior unchanged.
In `@lib/featbit/options.rb`:
- Around line 17-60: Update Options#initialize to centralize the shared fallback
values in default constants or a single reusable defaults structure, then use
those references for both the keyword defaults and rescue fallback assignments.
Remove the duplicated literals while preserving the existing safe fallback
behavior, including offline and disabled events.
In `@spec/client_spec.rb`:
- Around line 62-71: Update the “never raises from public evaluation methods”
example to pass a valid user, such as one with key “u1”, to both
client.variation calls so Evaluator#evaluate reaches broken_store.flag or
segment. Preserve the existing assertions that evaluation does not raise and
returns the fallback value.
In `@spec/data_store_spec.rb`:
- Around line 5-48: Add focused examples to FeatBit::InMemoryDataStore coverage
for archived filtering through flag, segment, and all_flags, verifying archived
items are excluded while active items remain available. Also exercise
upsert(:segments, ...) with newer, stale, and equal versions as applicable,
asserting the segment data and `@segment_versions` staleness behavior matches the
flags path.
In `@spec/event_processor_spec.rb`:
- Around line 121-127: Extend the “makes concurrent close idempotent” example
after the thread results assertion to call processor.close again and assert it
returns true, verifying the cached shutdown result is reused for a later
serialized call without re-invoking shutdown.
- Around line 18-33: Rewrite the “drops instead of blocking when the bounded
queue is full” spec to use a normally initialized EventProcessor instance rather
than described_class.allocate and manual instance-variable setup. Hold the
worker in a blocking sender or equivalent controlled state until its bounded
queue is saturated, then verify enqueue returns false and dropped_events equals
one while preserving the existing capacity and non-blocking behavior assertions.
In `@spec/web_socket_data_synchronizer_spec.rb`:
- Around line 28-48: Update the test around synchronizer.process_message to
include two patch entries for the same flag: deliver the older updatedAt and
value first, followed by the newer updatedAt and value, then assert the final
stored flag uses the newer value while changes still reports only "welcome".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: db2da935-619d-4627-8daf-76ce55092380
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
.github/workflows/ci.yml.gitignore.rubocop.ymlCHANGELOG.mdGemfileREADME.mdRakefiledocs/DESIGN.mddocs/REVIEW.mdexamples/Console/Gemfileexamples/Console/README.mdexamples/Console/app.rbexamples/Rails/Gemfileexamples/Rails/README.mdexamples/Rails/Rakefileexamples/Rails/app/controllers/application_controller.rbexamples/Rails/app/controllers/flags_controller.rbexamples/Rails/bin/railsexamples/Rails/config.ruexamples/Rails/config/application.rbexamples/Rails/config/boot.rbexamples/Rails/config/environment.rbexamples/Rails/config/environments/development.rbexamples/Rails/config/environments/production.rbexamples/Rails/config/environments/test.rbexamples/Rails/config/initializers/featbit.rbexamples/Rails/config/routes.rbexamples/Rails/lib/featbit_client_registry.rbfeatbit-server-sdk.gemspeclib/featbit.rblib/featbit/client.rblib/featbit/data_store.rblib/featbit/evaluation_detail.rblib/featbit/evaluator.rblib/featbit/event_processor.rblib/featbit/options.rblib/featbit/status.rblib/featbit/user.rblib/featbit/version.rblib/featbit/web_socket_data_synchronizer.rbscripts/live_integration_check.rbscripts/resource_audit.rbspec/client_spec.rbspec/data_store_spec.rbspec/evaluator_spec.rbspec/event_processor_spec.rbspec/examples_spec.rbspec/options_spec.rbspec/spec_helper.rbspec/web_socket_data_synchronizer_spec.rb
| matrix: | ||
| ruby: ["3.1", "3.4"] | ||
| steps: | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict and avoid persisting the workflow token.
actions/checkout leaves the token in local git config by default; later PR-controlled dependency installation can access it. Set read-only permissions and disable credential persistence.
Proposed fix
jobs:
test:
+ permissions:
+ contents: read
runs-on: ubuntu-latest
@@
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v5
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v5 | |
| jobs: | |
| test: | |
| permissions: | |
| contents: read | |
| runs-on: ubuntu-latest | |
| steps: | |
| - uses: actions/checkout@v5 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 16-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 16, Update the actions/checkout step in the
CI workflow to disable persisted credentials and restrict the workflow token to
read-only permissions. Configure checkout with persist-credentials disabled and
set the job or workflow permissions to only the minimum required read access,
preserving the existing checkout behavior.
Source: Linters/SAST tools
| when "MatchRegex" then !actual.nil? && Regexp.new(expected.to_s).match?(actual.to_s) | ||
| when "NotMatchRegex" then actual.nil? || !Regexp.new(expected.to_s).match?(actual.to_s) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the minimum Ruby version required by the featbit-server-sdk gemspec?
💡 Result:
There is no "featbit-server-sdk" for Ruby; therefore, no gemspec exists for a package by that name. FeatBit officially supports SDKs for.NET, Node.js, Python, Java, Go, JavaScript, React, and React Native [1][2]. While there is a Ruby gem named "feat-sdk" [3][4], it is associated with a different project (feat.so) [3][5], not FeatBit. That "feat-sdk" gem specifies a minimum Ruby version of 3.0 [3][4].
Citations:
- 1: https://github.com/featbit/featbit
- 2: https://github.com/featbit/featbit-skills
- 3: https://github.com/feathq/ruby-sdk/blob/main/feat-sdk.gemspec
- 4: https://rubygems.org/gems/feat-sdk
- 5: https://github.com/feathq/ruby-sdk/blob/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked gemspec/relevant files =="
git ls-files | awk '
/(^|\/)(Gemfile(\.lock)?|.*gemspec|lock\.ya?ml|pyproject\.toml|\.(rb)$)/ {print}
' | sed -n '1,200p'
echo
echo "== find evaluator/client files =="
fd -a 'evaluator\.rb|client\.rb' . | sed 's#^\./##'
echo
echo "== evaluator relevant lines =="
evaluator_file="$(fd 'evaluator\.rb' . | head -n1)"
if [ -n "${evaluator_file:-}" ]; then
wc -l "$evaluator_file"
sed -n '90,180p' "$evaluator_file" | cat -n
fi
echo
echo "== search for timeout/cache/client evaluation context =="
rg -n "Regexp\.new|Regexp\.timeout|timeout:|evaluate\(|Regexp::TimeoutError|MatchRegex|NotMatchRegex|Client|`#variation_detail`|variation_detail" -S .Repository: featbit/featbit-ruby-server-sdk
Length of output: 10426
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== gemspec =="
cat -n featbit-server-sdk.gemspec | sed -n '1,220p'
echo
echo "== Gemfile =="
cat -n Gemfile | sed -n '1,220p'
echo
echo "== user/public key access pattern =="
fd 'user\.rb' . | while read -r f; do
echo "--- $f"
cat -n "$f" | sed -n '1,220p'
done
echo
echo "== public custom attribute tests =="
cat -n spec/evaluator_spec.rb | sed -n '1,180p'Repository: featbit/featbit-ruby-server-sdk
Length of output: 7850
Bound regex evaluation ReDoS risk.
Regexp.new(expected.to_s) is called on every MatchRegex/NotMatchRegex match without a timeout, and expected comes from targeting config while actual comes from user-supplied attribute values. Set a process-level Regexp.timeout. If recompiling this pattern often is a hot-path concern, cache the bounded regex instead of creating a fresh one each evaluation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/evaluator.rb` around lines 138 - 139, Update the MatchRegex and
NotMatchRegex branches in the evaluator to enforce a process-level
Regexp.timeout before evaluating targeting patterns, while preserving their
existing nil and match semantics. Avoid unnecessary repeated Regexp.new
compilation in this hot path by reusing a bounded compiled pattern if an
appropriate cache already exists.
Source: Linters/SAST tools
| def close | ||
| @close_mutex.synchronize do | ||
| return @close_result unless @close_result.nil? | ||
|
|
||
| mark_closed | ||
| ack = Queue.new | ||
| @control_queue << { type: STOP, ack: ack } | ||
| delivered = Timeout.timeout(CONTROL_TIMEOUT) { ack.pop } != false | ||
| @thread&.join(CONTROL_TIMEOUT) | ||
| @close_result = delivered && !@thread&.alive? | ||
| end | ||
| rescue StandardError => e | ||
| @options.logger&.warn("FeatBit event processor close failed: #{e.message}") | ||
| false | ||
| end |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
close waits the full CONTROL_TIMEOUT when the worker never started.
If the constructor's rescue path ran (@thread nil, @closed true), nothing consumes the STOP message, so close blocks 5 s and returns false while leaving @close_result nil — every subsequent call repeats the 5 s wait. Short-circuit when there is no live worker.
🔧 Proposed fix
mark_closed
+ if `@thread.nil`? || !`@thread.alive`?
+ `@close_result` = true
+ return `@close_result`
+ end
+
ack = Queue.new📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def close | |
| @close_mutex.synchronize do | |
| return @close_result unless @close_result.nil? | |
| mark_closed | |
| ack = Queue.new | |
| @control_queue << { type: STOP, ack: ack } | |
| delivered = Timeout.timeout(CONTROL_TIMEOUT) { ack.pop } != false | |
| @thread&.join(CONTROL_TIMEOUT) | |
| @close_result = delivered && !@thread&.alive? | |
| end | |
| rescue StandardError => e | |
| @options.logger&.warn("FeatBit event processor close failed: #{e.message}") | |
| false | |
| end | |
| def close | |
| `@close_mutex.synchronize` do | |
| return `@close_result` unless `@close_result.nil`? | |
| mark_closed | |
| if `@thread.nil`? || !`@thread.alive`? | |
| `@close_result` = true | |
| return `@close_result` | |
| end | |
| ack = Queue.new | |
| `@control_queue` << { type: STOP, ack: ack } | |
| delivered = Timeout.timeout(CONTROL_TIMEOUT) { ack.pop } != false | |
| `@thread`&.join(CONTROL_TIMEOUT) | |
| `@close_result` = delivered && !`@thread`&.alive? | |
| end | |
| rescue StandardError => e | |
| `@options.logger`&.warn("FeatBit event processor close failed: #{e.message}") | |
| false | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/event_processor.rb` around lines 63 - 77, Update close to
short-circuit when `@thread` is nil or no longer alive, returning a successful
idempotent result without enqueueing STOP or waiting on the control queue.
Ensure `@close_result` is assigned on this path so subsequent close calls return
immediately, while preserving the existing shutdown flow for a live worker.
| def send_batch(batch) | ||
| return true if batch.empty? | ||
|
|
||
| batch.each_slice(@options.events_batch_size).all? { |slice| send_slice(slice) } | ||
| end |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
all? short-circuits and silently drops remaining slices.
If the first slice fails, all? stops iterating and the rest of the batch is never sent — those events are lost without a log or drop-counter increment. Attempt every slice, then reduce.
🔧 Proposed fix
- batch.each_slice(`@options.events_batch_size`).all? { |slice| send_slice(slice) }
+ batch.each_slice(`@options.events_batch_size`).map { |slice| send_slice(slice) }.all?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def send_batch(batch) | |
| return true if batch.empty? | |
| batch.each_slice(@options.events_batch_size).all? { |slice| send_slice(slice) } | |
| end | |
| def send_batch(batch) | |
| return true if batch.empty? | |
| batch.each_slice(`@options.events_batch_size`).map { |slice| send_slice(slice) }.all? | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/event_processor.rb` around lines 151 - 155, Update send_batch so
it attempts send_slice for every slice produced by batch.each_slice, rather than
using short-circuiting all?. Accumulate whether all slices succeeded while
continuing through the remaining slices, preserving the existing true result for
empty batches and returning the aggregate success status.
| def initialize(key, name: nil, custom: {}) | ||
| @key = key.to_s | ||
| @name = name | ||
| @custom = deep_freeze(deep_copy(custom || {}).transform_keys(&:to_s)) | ||
| freeze | ||
| rescue StandardError | ||
| @key = "" | ||
| @name = nil | ||
| @custom = {}.freeze | ||
| freeze | ||
| end |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
key/name are not defensively copied or frozen, unlike custom.
@key = key.to_s returns the same String object when key is already a String (no copy), and @name = name stores the raw reference. Only custom goes through deep_copy/deep_freeze. Freezing self doesn't freeze referenced objects, so a caller mutating the original key/name string after construction would leak through this "immutable" User.
🛡️ Proposed fix: reuse the existing deep_copy/deep_freeze helpers
def initialize(key, name: nil, custom: {})
- `@key` = key.to_s
- `@name` = name
+ `@key` = deep_freeze(deep_copy(key.to_s))
+ `@name` = deep_freeze(deep_copy(name))
`@custom` = deep_freeze(deep_copy(custom || {}).transform_keys(&:to_s))
freeze📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def initialize(key, name: nil, custom: {}) | |
| @key = key.to_s | |
| @name = name | |
| @custom = deep_freeze(deep_copy(custom || {}).transform_keys(&:to_s)) | |
| freeze | |
| rescue StandardError | |
| @key = "" | |
| @name = nil | |
| @custom = {}.freeze | |
| freeze | |
| end | |
| def initialize(key, name: nil, custom: {}) | |
| `@key` = deep_freeze(deep_copy(key.to_s)) | |
| `@name` = deep_freeze(deep_copy(name)) | |
| `@custom` = deep_freeze(deep_copy(custom || {}).transform_keys(&:to_s)) | |
| freeze | |
| rescue StandardError | |
| `@key` = "" | |
| `@name` = nil | |
| `@custom` = {}.freeze | |
| freeze | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/user.rb` around lines 7 - 17, Update User#initialize to
defensively copy and deeply freeze both key and name using the existing
deep_copy and deep_freeze helpers, matching the treatment of custom. Preserve
the existing nil behavior for name and ensure the resulting User cannot reflect
mutations to caller-owned string objects.
| rescue StandardError => e | ||
| fail_status(e, interrupted: true) | ||
| interruptible_sleep(delay) unless @closed | ||
| delay = [delay * 2, 30.0].min | ||
| ensure | ||
| safe_close_socket(socket) if @closed | ||
| @socket_mutex.synchronize { @socket = nil } | ||
| end |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Socket is leaked when the connection loop raises.
ensure only closes the socket when @closed is true. If configure_socket or socket.send raises (Line 95/100), the rescue path drops the only reference to an still-open socket and reconnects, so a reconnect storm accumulates live connections. Close the socket unconditionally when leaving the iteration.
🔧 Proposed fix
ensure
- safe_close_socket(socket) if `@closed`
+ safe_close_socket(socket) unless socket.nil?
`@socket_mutex.synchronize` { `@socket` = nil }
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rescue StandardError => e | |
| fail_status(e, interrupted: true) | |
| interruptible_sleep(delay) unless @closed | |
| delay = [delay * 2, 30.0].min | |
| ensure | |
| safe_close_socket(socket) if @closed | |
| @socket_mutex.synchronize { @socket = nil } | |
| end | |
| rescue StandardError => e | |
| fail_status(e, interrupted: true) | |
| interruptible_sleep(delay) unless `@closed` | |
| delay = [delay * 2, 30.0].min | |
| ensure | |
| safe_close_socket(socket) unless socket.nil? | |
| `@socket_mutex.synchronize` { `@socket` = nil } | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/web_socket_data_synchronizer.rb` around lines 110 - 117, Update
the ensure block in the connection loop to call safe_close_socket(socket)
unconditionally before clearing `@socket`, so sockets are closed whenever an
iteration exits, including configure_socket or send failures; preserve the
existing mutex-based `@socket` reset.
| def process_patch(data) | ||
| items = Array(fetch(data, "featureFlags", [])).map { |flag| [:flags, flag] } | ||
| items.concat(Array(fetch(data, "segments", [])).map { |segment| [:segments, segment] }) | ||
| changed_keys = [] | ||
| results = items.sort_by { |_kind, item| item_version(item) }.map do |kind, item| | ||
| applied = @data_store.upsert(kind, item, version: item_version(item)) | ||
| if applied && kind == :flags | ||
| changed_keys << fetch(item, "key").to_s | ||
| elsif applied | ||
| changed_keys.concat(flags_referencing_segment(fetch(item, "id").to_s)) | ||
| end | ||
| applied | ||
| end | ||
| [results.all?, changed_keys.uniq] | ||
| end |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Partial patch application suppresses listener notifications.
results.all? makes the whole patch report changed = false when any single item is stale (upsert returns false). Line 71 then returns early, so flags that were applied never notify listeners and the status is not moved to READY. Stale items in a patch are normal, so treat "any applied" as changed.
🔧 Proposed fix
- [results.all?, changed_keys.uniq]
+ [results.any?, changed_keys.uniq]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def process_patch(data) | |
| items = Array(fetch(data, "featureFlags", [])).map { |flag| [:flags, flag] } | |
| items.concat(Array(fetch(data, "segments", [])).map { |segment| [:segments, segment] }) | |
| changed_keys = [] | |
| results = items.sort_by { |_kind, item| item_version(item) }.map do |kind, item| | |
| applied = @data_store.upsert(kind, item, version: item_version(item)) | |
| if applied && kind == :flags | |
| changed_keys << fetch(item, "key").to_s | |
| elsif applied | |
| changed_keys.concat(flags_referencing_segment(fetch(item, "id").to_s)) | |
| end | |
| applied | |
| end | |
| [results.all?, changed_keys.uniq] | |
| end | |
| def process_patch(data) | |
| items = Array(fetch(data, "featureFlags", [])).map { |flag| [:flags, flag] } | |
| items.concat(Array(fetch(data, "segments", [])).map { |segment| [:segments, segment] }) | |
| changed_keys = [] | |
| results = items.sort_by { |_kind, item| item_version(item) }.map do |kind, item| | |
| applied = `@data_store.upsert`(kind, item, version: item_version(item)) | |
| if applied && kind == :flags | |
| changed_keys << fetch(item, "key").to_s | |
| elsif applied | |
| changed_keys.concat(flags_referencing_segment(fetch(item, "id").to_s)) | |
| end | |
| applied | |
| end | |
| [results.any?, changed_keys.uniq] | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/featbit/web_socket_data_synchronizer.rb` around lines 192 - 206, The
process_patch method incorrectly reports a partially applied patch as unchanged
because it returns results.all?. Replace the aggregate status with an “any item
applied” result, while preserving changed_keys collection and deduplication so
successfully upserted flags or segments notify listeners and allow the
synchronizer to reach READY even when other items are stale.
| - [Console](/examples/Console) | ||
| - [Rails](/examples/Rails) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use repository-relative documentation links.
These links resolve outside this repository when rendered on GitHub. Use ./examples/... and ./docs/... so users can open the bundled examples and review guide.
Proposed fix
-- [Console](/examples/Console)
-- [Rails](/examples/Rails)
+- [Console](./examples/Console)
+- [Rails](./examples/Rails)
@@
-See [docs/REVIEW.md](/docs/REVIEW.md) for the code-review findings, fixes, and verification matrix.
+See [docs/REVIEW.md](./docs/REVIEW.md) for the code-review findings, fixes, and verification matrix.Also applies to: 256-256
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 72 - 73, Update the README documentation links for
the Console and Rails examples to use repository-relative paths prefixed with
./examples/ instead of root-relative paths. Apply the same relative-link
convention to the review guide link referenced elsewhere in the README, using
./docs/ so all links remain within the repository on GitHub.
| detail = client.variation_detail(flag_key, user, false) | ||
| raise "live flag evaluation failed: #{detail.error_message}" unless detail.success? | ||
|
|
||
| errors = Queue.new | ||
| thread_count = Integer(ENV.fetch("FEATBIT_LIVE_THREADS", "8"), 10) | ||
| evaluations_per_thread = Integer(ENV.fetch("FEATBIT_LIVE_EVALUATIONS_PER_THREAD", "5"), 10) | ||
| evaluation_count = thread_count * evaluations_per_thread | ||
| thread_count.times.map do |index| | ||
| Thread.new do | ||
| thread_user = FeatBit::User.new("ruby-live-review-#{index}") | ||
| evaluations_per_thread.times do | ||
| value = client.bool_variation(flag_key, thread_user, false) | ||
| errors << "non-boolean value" unless [true, false].include?(value) | ||
| rescue StandardError => e | ||
| errors << e.message | ||
| end | ||
| end | ||
| end.each(&:join) | ||
| raise "concurrent live evaluations failed: #{errors.size}" unless errors.empty? |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a boolean baseline flag result.
A successful variation_detail may return a non-boolean value. Then bool_variation falls back to false, which passes Line 39 and falsely reports successful concurrent boolean evaluation. Validate the initial result type before starting the threads.
Proposed fix
detail = client.variation_detail(flag_key, user, false)
-raise "live flag evaluation failed: #{detail.error_message}" unless detail.success?
+unless detail.success? && [true, false].include?(detail.value)
+ raise "live boolean flag evaluation failed: #{detail.error_message || "unexpected flag type"}"
+end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| detail = client.variation_detail(flag_key, user, false) | |
| raise "live flag evaluation failed: #{detail.error_message}" unless detail.success? | |
| errors = Queue.new | |
| thread_count = Integer(ENV.fetch("FEATBIT_LIVE_THREADS", "8"), 10) | |
| evaluations_per_thread = Integer(ENV.fetch("FEATBIT_LIVE_EVALUATIONS_PER_THREAD", "5"), 10) | |
| evaluation_count = thread_count * evaluations_per_thread | |
| thread_count.times.map do |index| | |
| Thread.new do | |
| thread_user = FeatBit::User.new("ruby-live-review-#{index}") | |
| evaluations_per_thread.times do | |
| value = client.bool_variation(flag_key, thread_user, false) | |
| errors << "non-boolean value" unless [true, false].include?(value) | |
| rescue StandardError => e | |
| errors << e.message | |
| end | |
| end | |
| end.each(&:join) | |
| raise "concurrent live evaluations failed: #{errors.size}" unless errors.empty? | |
| detail = client.variation_detail(flag_key, user, false) | |
| unless detail.success? && [true, false].include?(detail.value) | |
| raise "live boolean flag evaluation failed: #{detail.error_message || "unexpected flag type"}" | |
| end | |
| errors = Queue.new | |
| thread_count = Integer(ENV.fetch("FEATBIT_LIVE_THREADS", "8"), 10) | |
| evaluations_per_thread = Integer(ENV.fetch("FEATBIT_LIVE_EVALUATIONS_PER_THREAD", "5"), 10) | |
| evaluation_count = thread_count * evaluations_per_thread | |
| thread_count.times.map do |index| | |
| Thread.new do | |
| thread_user = FeatBit::User.new("ruby-live-review-#{index}") | |
| evaluations_per_thread.times do | |
| value = client.bool_variation(flag_key, thread_user, false) | |
| errors << "non-boolean value" unless [true, false].include?(value) | |
| rescue StandardError => e | |
| errors << e.message | |
| end | |
| end | |
| end.each(&:join) | |
| raise "concurrent live evaluations failed: #{errors.size}" unless errors.empty? |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/live_integration_check.rb` around lines 27 - 45, Validate that the
successful result from variation_detail is a boolean before starting concurrent
evaluations. Update the initial flag evaluation around detail and
detail.success? to reject non-boolean values, while preserving the existing
error handling and thread checks.
| RSpec.describe FeatBit::Evaluator do | ||
| let(:store) { FeatBit::InMemoryDataStore.new } | ||
| let(:evaluator) do | ||
| described_class.new(flag_getter: ->(key) { store.flag(key) }, segment_getter: ->(id) { store.segment(id) }) | ||
| end |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
No test exercises the actual percentage-bucketing math.
Every rollout range in this spec is [0, 1], which short-circuits in_bucket? before the MD5-hash computation ever runs. Given the bucketing algorithm is the crux of correct/consistent flag targeting (and its cross-SDK parity is unverified, per the evaluator.rb review), add at least one test with a partial range (e.g. [0, 0.5]) asserting a specific known user key lands in a specific variation, to lock in current behavior and catch future regressions in the hashing math.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/evaluator_spec.rb` around lines 5 - 9, Add a percentage-rollout example
in the FeatBit::Evaluator specs using a partial range such as [0, 0.5], with a
fixed user key and an assertion for its expected variation. Ensure the case
exercises the MD5-based bucketing path in in_bucket? and preserves the existing
evaluator setup and full-range tests.
1 participant
Summary
Reliability fixes covered
Validation
Summary by CodeRabbit
New Features
Documentation
Quality