Feature flag clean up

[v-robaiken]

What are you trying to accomplish?

Make the documented three-day cooldown apply when a version-update job has no cooldown block. Currently, Job#build_cooldown returns nil for an absent block, so update checkers receive no cooldown and can select releases published less than three days ago.

This change constructs the default three-day cooldown when configuration is absent, while preserving an explicit default-days: 0 opt-out and the existing security-update bypass. It also removes obsolete nil handling from cooldown telemetry now that version-update jobs always have effective cooldown options.

Anything you want to highlight for special attention from reviewers?

The default is applied in Core rather than synthesizing a cooldown block in the API job payload. This keeps explicit configuration parsing unchanged and ensures CLI/local jobs receive the same default behavior. Security update paths continue passing nil to update checkers, so security fixes are not delayed.

How will you know you've accomplished your goal?

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[Copilot]

Pull request overview

Applies the default three-day cooldown when jobs omit cooldown configuration while preserving opt-outs and security-update bypasses.

Changes:

• Makes Job#cooldown always return cooldown options.
• Simplifies cooldown telemetry handling.
• Updates tests for default propagation.

Show a summary per file

File	Description
updater/lib/dependabot/job.rb	Builds default cooldown options.
updater/lib/dependabot/api_client.rb	Removes obsolete nullable cooldown handling.
updater/spec/dependabot/job_spec.rb	Tests implicit cooldown defaults.
updater/spec/dependabot/api_client_spec.rb	Updates telemetry nil-case terminology.
updater/spec/dependabot/updater_spec.rb	Expects default cooldown propagation.
updater/spec/dependabot/updater/operations/update_all_versions_spec.rb	Tests defaults and security bypass behavior.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (4)

updater/lib/dependabot/job.rb:701

• build_cooldown returns a ReleaseCooldownOptions initialized with only default_days when cooldown is not configured, but elsewhere (e.g., telemetry) the code reads semver_major_days/minor_days/patch_days. To avoid relying on implicit defaults inside ReleaseCooldownOptions, initialize the object with all cooldown periods set to default_cooldown_days (default/major/minor/patch) when no cooldown config is provided.

      unless cooldown
        return Dependabot::Package::ReleaseCooldownOptions.new(
          default_days: default_cooldown_days
        )
      end

updater/lib/dependabot/job.rb:42

• This comment is now misleading: the code applies a default cooldown even when the entire cooldown block is not configured (not only when default-days is missing). Update the comment to reflect that the default applies when cooldown is absent and/or when default-days is not set.

    # Default cooldown period (in days) applied when `default-days` is not configured.

updater/lib/dependabot/api_client.rb:345

• The string literal \"cli\" is a magic value controlling telemetry behavior. Consider extracting it into a named constant (e.g., CLI_JOB_ID) or a predicate method (e.g., cli_mode?) to make the intent clearer and reduce the risk of mismatches if the identifier ever changes.

      # The CLI's local API does not implement this hosted-service telemetry endpoint.
      return if job.nil? || job_id == "cli"

updater/spec/dependabot/updater/operations/update_all_versions_spec.rb:396

• This new spec only asserts default_days, but the behavior change (and job_spec) implies all cooldown periods default (major/minor/patch too). Strengthen the assertion to also check semver_major_days, semver_minor_days, and semver_patch_days so the test will catch regressions where only default_days is set.

        expect(stub_update_checker_class).to have_received(:new).with(
          hash_including(
            update_cooldown: having_attributes(default_days: Dependabot::Job::DEFAULT_COOLDOWN_DAYS)
          )
        )

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (5)

updater/lib/dependabot/job.rb:1

• In the cooldown-nil path, only default_days is set, but downstream code (e.g., telemetry payload) reads semver_major_days/minor_days/patch_days without nil-handling. To avoid inconsistent defaults between the nil and configured paths, initialize the options with all cooldown periods (default + semver-specific) or centralize construction so both paths produce fully-populated ReleaseCooldownOptions.
  updater/spec/dependabot/updater_spec.rb:1
• The helper currently uses a parameter named update_cooldown but maps it to cooldown: when building the job, which is confusing and easy to misuse. Consider renaming the helper argument to cooldown (or cooldown_definition) and defaulting it to nil so tests that don’t care about cooldown exercise the real defaulting behavior; tests that need deterministic behavior can explicitly pass { \"default-days\" => 0 }.
  updater/spec/dependabot/updater_spec.rb:1
• The helper currently uses a parameter named update_cooldown but maps it to cooldown: when building the job, which is confusing and easy to misuse. Consider renaming the helper argument to cooldown (or cooldown_definition) and defaulting it to nil so tests that don’t care about cooldown exercise the real defaulting behavior; tests that need deterministic behavior can explicitly pass { \"default-days\" => 0 }.
  updater/spec/dependabot/updater_spec.rb:1
• Hard-coding default_days: 0 here couples the spec to a specific fixture/helper default and can conflict with the newly introduced default cooldown behavior (DEFAULT_COOLDOWN_DAYS). If the intent is merely to assert that a cooldown object is passed, prefer matching the type/shape (e.g., an_instance_of(...) or having_attributes(default_days: Dependabot::Job::DEFAULT_COOLDOWN_DAYS) depending on scenario) rather than a specific numeric value.
  updater/lib/dependabot/api_client.rb:1
• Using the magic string \"cli\" as a sentinel for disabling hosted telemetry makes this behavior implicit and harder to maintain. Consider introducing an explicit constructor flag (e.g., hosted_telemetry: false) or a predicate (e.g., cli_mode?) and reusing it here, so the decision isn’t tied to a particular job_id value.

• Files reviewed: 65/65 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (5)

updater/lib/dependabot/api_client.rb:1

• record_cooldown_meta no longer has a guard to prevent sending hosted-service telemetry when running via the Dependabot CLI, but the updated spec expects no request for update_job_id/job id \"cli\". Add an explicit early-return (or predicate) to skip record_cooldown_meta when the client is instantiated for CLI runs (e.g., when the update job id is \"cli\", or when a dedicated hosted?/telemetry_enabled? flag is false).
  updater/lib/dependabot/api_client.rb:1
• record_cooldown_meta no longer has a guard to prevent sending hosted-service telemetry when running via the Dependabot CLI, but the updated spec expects no request for update_job_id/job id \"cli\". Add an explicit early-return (or predicate) to skip record_cooldown_meta when the client is instantiated for CLI runs (e.g., when the update job id is \"cli\", or when a dedicated hosted?/telemetry_enabled? flag is false).
  updater/lib/dependabot/job.rb:1
• When cooldown is not configured, build_cooldown constructs ReleaseCooldownOptions with only default_days. However, the updated job_spec asserts that semver_major_days, semver_minor_days, and semver_patch_days also default to DEFAULT_COOLDOWN_DAYS. Update the non-configured branch to populate those semver fields explicitly (or construct via the same to_options pathway) so the object is fully defaulted and consistent with expectations.
  updater/spec/dependabot/updater_spec.rb:1
• The helper parameter is named update_cooldown but is passed into Dependabot::Job as cooldown. Renaming the parameter to cooldown (and adjusting call sites) would make the helper align with Dependabot::Job’s API and reduce confusion between cooldown vs update_cooldown terminology.
  updater/spec/dependabot/updater_spec.rb:1
• The helper parameter is named update_cooldown but is passed into Dependabot::Job as cooldown. Renaming the parameter to cooldown (and adjusting call sites) would make the helper align with Dependabot::Job’s API and reduce confusion between cooldown vs update_cooldown terminology.

• Files reviewed: 65/65 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (4)

updater/lib/dependabot/api_client.rb:1

• The new spec expects record_cooldown_meta to not send hosted-service telemetry when running via the CLI, but the implementation now only skips when job is nil. As written, a CLI-initialized client will still POST cooldown telemetry whenever a job is present. Add an explicit guard for the CLI case (e.g., based on the update job id / mode passed into the client) so record_cooldown_meta becomes a no-op for CLI runs.
  updater/lib/dependabot/job.rb:1
• When cooldown is not configured, build_cooldown only sets default_days and relies on ReleaseCooldownOptions to implicitly populate semver_*_days. Given the specs assert all semver cooldown periods default to DEFAULT_COOLDOWN_DAYS, it's more robust to set semver_major_days, semver_minor_days, and semver_patch_days explicitly here (or centralize the defaulting inside ReleaseCooldownOptions) to avoid coupling to initializer defaults.
  updater/spec/dependabot/updater_spec.rb:1
• In this helper, the keyword argument is named update_cooldown but it is passed through as cooldown:. Renaming the helper parameter to cooldown (or cooldown_definition) would make the intent clearer and reduce confusion now that the production attribute is cooldown rather than update_cooldown.
  updater/spec/dependabot/updater_spec.rb:1
• In this helper, the keyword argument is named update_cooldown but it is passed through as cooldown:. Renaming the helper parameter to cooldown (or cooldown_definition) would make the intent clearer and reduce confusion now that the production attribute is cooldown rather than update_cooldown.

• Files reviewed: 65/65 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

Comments suppressed due to low confidence (3)

updater/lib/dependabot/job.rb:1

• When cooldown is not configured, this builds ReleaseCooldownOptions with only default_days set. That makes the behavior of semver_major_days/minor_days/patch_days depend on ReleaseCooldownOptions' internal defaults, which is less explicit and can be brittle if that class changes. Consider initializing all cooldown periods here (default + semver major/minor/patch) to default_cooldown_days so the fallback behavior is fully defined in Job.
  updater/spec/dependabot/updater_spec.rb:1
• The helper argument is named update_cooldown, but the production attribute appears to be cooldown (and the helper later passes it as cooldown:). Renaming this helper parameter to cooldown (and updating call sites) would reduce confusion and better align with the rest of the codebase terminology.
  updater/lib/dependabot/api_client.rb:1
• Using the magic string \"cli\" as a sentinel for skipping telemetry makes this behavior harder to discover and easier to break if the identifier changes. Consider extracting this into a constant (e.g., CLI_JOB_ID = \"cli\") or a predicate (e.g., cli?) so the decision is centralized and self-documenting.

• Files reviewed: 65/65 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

• Files reviewed: 65/65 changed files
• Comments generated: 0 new
• Review effort level: Medium