Skip to content

AI Feature flag removal

What does this MR do and why?

This MR introduces a new Housekeeper Keep to remove old feature flags from the code. It does this by:

  1. Iterate over all feature flag YML definitions which are more than 12 milestones old
  2. It then checks in the associated rollout issue for comments indicating whether or not it enabled
  3. It greps the codebase for references to the feature flag
  4. It then passes the file to claude and asks Claude to remove the feature flag in the form of a diff. Claude is told whether the feature flag is enabled or disabled and should remove the feature flag depending in such a way that the new behaviour matches the feature flag state
  5. It applies the diff to the file
  6. It then creates an MR (housekeper handles this)

TODO

  1. Deal with javascript feature flag usage (camelCased) . Will require updating the regex to find JS files and also we need a custom prompt to explain to the AI how to remove the JS references.
  2. Fix or removed the aborted logic
    1. This was added to deal with the problem of running the keep with multiple changes at once. Without the abort option housekeeper will always try to create an MR for every change we yield. We can choose to skip the yield(change) when we know that the AI did not succeed but this brings a different problem if we've made some edits but not edited everything. Because we also rely on yield(change) to have housekeeper undo all the changes that we've made. So I was noticing problems here and I introduced abort but in my testing it didn't seem to be working and I was regularly left with extra files in my diff after running the keep when it aborted changes.
  3. We should also abort a change if ::Gitlab::Housekeeper::Shell.rubocop_autocorrect(file) fails. Right now it raises and stops running housekeeper. But it would be ideal if we could run housekeeper with -m 10 and it would just skip any changes that failed for some reason and move on to the next change. This probably requires abort to work correctly or some other way to catch these errors and undo all our changes and continue on.
  4. Add an additional check after all changes are made to grep the codebase one more time. We should abort the change if we find any remaining references to the feature flag
  5. Extract the abort and filter_identifiers improvements into a separate MR to be merged first as they are general improvements . They probably also need tests
  6. Run keep for 5 MRs and figure out what problems it's having and add more TODOs for those problems
  7. Keep iterating until this is more reliable. It doesn't always have to succesfully remove every feature flag but we want to minimize noise and not create too many MRs that are wrong and someone has to take the time to review them.
  8. Once this keep is reliable we can merge it and start running it in CI
  9. Update the keep to work out which feature flags are enabled on GitLab.com instead of just relying on the YAML default_enabled: true. This is necessary because most of the time people aren't even updating the YAML file once they enable it via ChatOps. This will also require us to figure out how to get this data somehow from GitLab. We can do this using the Postgres AI approach we used for ::Keeps::OverdueFinalizeBackgroundMigration as the feature flag state is in the database. But it would be nice if it was easier for people to run this without production DB access. We could maybe also get this data from prometheus which is slightly lower risk and easier to add to our CI environment. Alternatively we could extract the data from comments on the rollout issues as these are also commenting every time the feature flag is changed.
  10. Add it to CI similar to what is done in gitlab-org/quality/engineering-productivity/team!238 (diffs)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Related to #450442 (closed)

Edited by Dylan Griffith

Merge request reports

Loading