Skip to content

fix(flags): report feature flag review dates before they break CI - #15973

Merged
teamleaderleo merged 7 commits into
mainfrom
parity/flag-review-report
Sep 30, 2026
Merged

teamleaderleo merged 7 commits into
mainfrom
parity/flag-review-report

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Feature flag review dates currently fail the feature-flags lint only after a date has already passed. A missed reviewBy date can turn main and every open PR red on the same day. This change adds an always-green lead-time report, a daily issue workflow, and a shared collector for the web and Swift registries. It also reports malformed calendar dates instead of crashing with a traceback.

.github/workflows/ci-guards.yml explains why this is report-only:

The conventions lint scans the whole repository, so gating a PR on it directly turns every open PR red whenever main carries one unrelated violation (#10409). Judge the change instead: report only violations absent from the base.

The lead-time check follows that policy. It reports dates within 30 days and the scheduled workflow opens or updates one issue without making pull requests fail over repository state they did not introduce.

The regression was kept in two commits as required:

  • Commit 0759abd6b81 added the malformed-date regression. python3 tests/test_lint_feature_flags_scope.py was red with ValueError: month must be in 1..12, not 13.
  • Commit ea39a4ab3fa guarded date.fromisoformat, added collect_flags(), and added the web plus Swift collector coverage. The same command was green: 6 tests passed.

The review dates were extended separately in cmux#15922.

Verification

  • python3 scripts/lint-feature-flags.py passed with 18 declarations.
  • python3 tests/test_lint_feature_flags_scope.py passed, 6 tests.
  • python3 tests/test_feature_flag_review_lead_time.py passed, 4 tests.
  • python3 tests/test_feature_flag_review_drift_issue.py passed, 2 tests.
  • python3 tests/test_ci_test_execution_registry.py passed, 29 tests.
  • python3 scripts/verify-local.py passed all applicable checks; Swift syntax had zero selected files, and native compilation, app tests, and app launch were not checked for this Python and YAML change.
  • actionlint .github/workflows/feature-flag-review-drift.yml passed.
  • python3 scripts/ci/run_ci_guards.py --group ci --keep-going --no-stamp ran; the unrelated test_seed_derived_data.py failures are host-sensitive low-disk prune tests that do not establish the low-disk condition. They are being fixed separately.

Changelog

none

🤖 Generated with Claude Code

teamleaderleo and others added 4 commits September 30, 2026 04:57
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 715cf045-8d90-4497-9e01-fd81f7919b42

📥 Commits

Reviewing files that changed from the base of the PR and between 6d7ad14 and 087ec6d.

📒 Files selected for processing (9)
  • .github/workflows/ci-guards.yml
  • .github/workflows/feature-flag-review-drift.yml
  • scripts/ci/workflow_guard_groups.py
  • scripts/lint-feature-flags.py
  • scripts/report-feature-flag-review-lead-time.py
  • tests/test-execution.toml
  • tests/test_feature_flag_review_drift_issue.py
  • tests/test_feature_flag_review_lead_time.py
  • tests/test_lint_feature_flags_scope.py

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.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo

teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Verification completed:

  • python3 scripts/lint-feature-flags.py passed with 18 declarations.
  • python3 tests/test_lint_feature_flags_scope.py passed, 6 tests.
  • python3 tests/test_feature_flag_review_lead_time.py passed, 4 tests.
  • python3 tests/test_feature_flag_review_drift_issue.py passed, 2 tests.
  • python3 tests/test_ci_test_execution_registry.py passed, 29 tests.
  • python3 scripts/verify-local.py passed all applicable checks; Swift syntax had zero selected files, and native compilation, app tests, and app launch were not checked.
  • actionlint .github/workflows/feature-flag-review-drift.yml passed.
  • python3 scripts/ci/run_ci_guards.py --group ci --keep-going --no-stamp ran. The reusable guard workflow structure step passed. The remaining failures were inherited from untouched tests/test_ci_main_full_suite.py and tests/test_seed_derived_data.py, and those files have no diff from origin/main.

The known main branch macOS compile admission failure is also inherited. Main currently has the backwards vendor/bonsplit pointer from #15747, while Sources/TerminalSharingDisplay.swift and Sources/TerminalSizeBoundsOverlayView.swift need symbols from the newer bonsplit. cmux#15930 is the separate pin fix. This PR changes Python and YAML only, so I am leaving that failure untouched.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Dogfood tours of 087ec6dd

sidebar-and-chrome-tour at 087ec6dd: not run

skipped: CI built this head on a runner pool whose products the UI test Macs cannot load, and media never compiles one; gh workflow run pr-media.yml -f pr=&lt;n&gt; -f allow_compile=true does

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

teamleaderleo and others added 3 commits September 30, 2026 06:07
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Follow-up verification after the scheduled-path repair:

python3 tests/test_feature_flag_review_lead_time.py

.........
----------------------------------------------------------------------
Ran 9 tests in 0.012s

OK

python3 tests/test_feature_flag_review_drift_issue.py

......
----------------------------------------------------------------------
Ran 6 tests in 0.203s

OK

python3 tests/test_lint_feature_flags_scope.py

........
----------------------------------------------------------------------
Ran 8 tests in 0.597s

OK

python3 tests/test_ci_test_execution_registry.py

.............................
----------------------------------------------------------------------
Ran 29 tests in 1.494s

OK

python3 scripts/report-feature-flag-review-lead-time.py

key                                         source file                           reviewBy    days remaining
------------------------------------------  ------------------------------------  ----------  --------------
agent-chat-ui-enabled-release               Sources/FeatureFlags.swift            2026-10-01  1             
cloud-machines-enabled-release              Sources/CmuxFeatureFlags+Cloud.swift  2026-10-01  1             
cloud-machines-enabled-release              Sources/FeatureFlags.swift            2026-10-01  1             
computer-use-ux-enabled-release             Sources/FeatureFlags.swift            2026-10-01  1             
mobile-connect-button-enabled-release       Sources/FeatureFlags.swift            2026-10-01  1             
mobile-task-composer-enabled-release        Sources/FeatureFlags.swift            2026-10-01  1             
mobile-workspace-changes-enabled-release    Sources/FeatureFlags.swift            2026-10-01  1             
pro-upgrade-ui-enabled-release              Sources/FeatureFlags.swift            2026-10-01  1             
pro-upgrade-ui-enabled-release              web/app/lib/feature-flags.ts          2026-10-01  1             
sidebar-account-button-enabled-release      Sources/FeatureFlags.swift            2026-10-01  1             
sidebar-appkit-list-experiment              Sources/FeatureFlags.swift            2026-10-01  1             
sidebar-workspace-agent-spinner-experiment  Sources/FeatureFlags.swift            2026-10-01  1             
simulator-enabled-release                   Sources/FeatureFlags.swift            2026-10-01  1             
workspace-todo-controls-enabled-release     Sources/FeatureFlags.swift            2026-10-01  1             

python3 scripts/report-feature-flag-review-lead-time.py --json

[
  {
    "daysRemaining": 1,
    "key": "agent-chat-ui-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "cloud-machines-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/CmuxFeatureFlags+Cloud.swift"
  },
  {
    "daysRemaining": 1,
    "key": "cloud-machines-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "computer-use-ux-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "mobile-connect-button-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "mobile-task-composer-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "mobile-workspace-changes-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "pro-upgrade-ui-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "pro-upgrade-ui-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "web/app/lib/feature-flags.ts"
  },
  {
    "daysRemaining": 1,
    "key": "sidebar-account-button-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "sidebar-appkit-list-experiment",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "sidebar-workspace-agent-spinner-experiment",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "simulator-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  },
  {
    "daysRemaining": 1,
    "key": "workspace-todo-controls-enabled-release",
    "reviewBy": "2026-10-01",
    "source": "Sources/FeatureFlags.swift"
  }
]

actionlint .github/workflows/iroh-v2-production-drift.yml .github/workflows/feature-flag-review-drift.yml

Actionlint produced no output and exited 0.

Report script routing:

linux_guard_tests=true
linux_guard_history=false
linux_guard_cli=false
linux_guard_source=false
ghosttykit_release=false
linux_guard_test_groups=["preflight","ci"]

Workflow routing:

linux_guard_tests=true
linux_guard_history=false
linux_guard_cli=false
linux_guard_source=false
ghosttykit_release=false
linux_guard_test_groups=["ci"]

Broken seam demonstration. With collect_flags temporarily renamed so the path import cannot find it, python3 scripts/report-feature-flag-review-lead-time.py --json produced:

{"error": "Feature flag review report failed: module 'lint_feature_flags' has no attribute 'collect_flags'"}

The command exited 0, and the payload is an error object rather than [], so the workflow leaves or opens the tracking issue instead of closing it.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: a subagent reviewed this branch at 575b3489e05 and found three blocking problems, all in the same shape. The report inverted at the cliff, a failure read as good news, and a routing omission removed the only test that would have caught either one.

  1. build_report gated on 0 <= days_remaining <= LEAD_TIME_DAYS, so a flag whose reviewBy had already passed fell out of the report. An expired flag is the case the report exists for, and dropping it made the report shrink to empty, which the scheduled workflow reads as an all-clear and uses to close the issue. test_already_passed_date_is_not_at_risk asserted that behaviour, so the suite defended the bug.
  2. The report catches every exception so it cannot fail a build, and in --json mode it printed []. [] is the all-clear payload. Any collector breakage therefore closed the drift issue and reported that all review dates were comfortable. python3 ... --json | tee file also sent a non-zero Python exit into tee, so the step stayed green either way.
  3. PATH_OWNERS in scripts/ci/workflow_guard_groups.py did not name scripts/report-feature-flag-review-lead-time.py, the new workflow, or its test, so a change to any of them ran no guard group.

Fixed: in 3d187c1eb73 (the failing regression), 7f74ee63cad and 087ec6dd883.

  • The lower bound is gone. Expired flags stay in the report with a negative daysRemaining, and the issue body now carries a Status column that says Already expired or Approaching per row. test_already_passed_date_is_not_at_risk is inverted to test_already_passed_date_stays_at_risk, with test_expired_flag_has_negative_days_remaining beside it.
  • The failure payload is now {"error": ...}, an object rather than an array. The github-script step wraps the read in try/catch, rejects anything that is not an array, and on failure comments on the open issue or opens one, then returns without closing anything. test_failed_payloads_never_close_an_open_issue covers a missing file, unparseable text and the error object; test_report_main_error_payload_is_rejected_by_workflow feeds the script's actual output through the workflow rather than a hand-written string. The pipe is now >.
  • PATH_OWNERS gains the report script, the workflow and its test, following the iroh-v2-production-drift.yml entries already there.
  • The empty-report comment no longer claims every date has more than 30 days remaining; it says no valid dates are approaching or expired and points at the linter for malformed ones.
  • collect_flags() returns (flags, registry_paths) so swift_registry_files() runs once, with test_main_discovers_registries_only_once pinning it. Registry reads use errors="replace", and a flag with no key or source renders as <missing key> / <missing source> instead of raising a TypeError in the sort. The _load_linter() seam is now covered directly.
  • The two tests that printed linter output now redirect stdout.

Verification on 087ec6dd883:

  • python3 tests/test_feature_flag_review_lead_time.py passed, 9 tests.
  • python3 tests/test_feature_flag_review_drift_issue.py passed, 6 tests.
  • python3 tests/test_lint_feature_flags_scope.py passed, 8 tests.

The counts in the Verification section above predate these commits; 6, 4 and 2 there are now 8, 9 and 6.

Left: nothing from the review. Two judgements recorded rather than changed. The report still exits 0 on every failure, which is deliberate so a collector bug cannot fail an unrelated build; the failure is now visible through the issue instead of the exit code. And expired dates keep failing the feature flag linter, so the issue is a reminder rather than the gate.

🤖 Generated with Claude Code

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: LAND. Reviewed with a subagent on the exact diff.

It fixes a genuine crash. On main, a reviewBy that matches DATE_RE but is not a calendar date reached datetime.date.fromisoformat(review) unguarded, so 2026-13-01 raised ValueError and the linter died with a traceback instead of reporting a policy violation. The PR wraps it and emits "'{key}' needs reviewBy: YYYY-MM-DD". Checked against both versions.

Boundaries are right at every edge. With LEAD_TIME_DAYS = 30 and days_remaining <= LEAD_TIME_DAYS: today reports, tomorrow reports, exactly 30 days out reports, 31 days out does not, past dates report and separately hard-fail the linter, malformed or absent are skipped by the report and hard-failed by the linter. So a flag whose date is today is warned about today and reddens CI tomorrow, since the linter's condition is review_date < today. That matches the stated intent. The warning and failure split is structural: the report wraps everything in except Exception and always returns 0, so it can never redden a build; the linter is the only thing that fails.

The body's claim that expired dates already fail the linter holds on PRs. ci.yml:1111 runs python3 scripts/verify-local.py in static-preflight with no if: excluding pull requests, and the bare invocation auto-selects feature-flags.

Two notes, neither blocking. Both scripts use datetime.date.today(), the runner's local date, which is UTC in CI but means a contributor west of UTC sees the flip up to a day before CI does, and the drift job's cron "17 9 * * *" is UTC. One line fixes it: datetime.datetime.now(datetime.timezone.utc).date(). Separately, test_feature_flag_review_drift_issue.py extracts the real inline JS from the workflow YAML and runs it under node -e with check=True, which is exactly the right kind of test, but there is no setup-node step in workflow-guard-tests, so it relies on node being preinstalled and a missing node is a hard failure rather than a skip. A shutil.which("node") guard and an anchored extraction would make it robust. I did not verify the Blacksmith ubuntu image carries node.

Mutation: LEAD_TIME_DAYS 30 to 0 fails the lead-time suite; removing the try/except ValueError fails the linter scope suite on the 2026-13-01 case; making the drift script close on a non-array payload fails the drift suite. All three pass unmutated (9, 6, 6 tests). Behaviour coverage, not shape coverage.

Wiring: both new guards run on pull requests under matrix.group == 'ci', and unlike #15988 this PR adds the PATH_OWNERS entries that make the ci group get selected for lint-feature-flags.py and report-feature-flag-review-lead-time.py. Both test files are registered in test-execution.toml. The drift workflow itself is schedule plus workflow_dispatch only, correct since it writes issues, with minimal permissions and cancel-in-progress: false, which is right for a per-day publisher.

Fixed: nothing needed. Left: the two notes. Auto-merge on.
— Raindrop g2 🫧 / Run: run_worker_20260930_3fc64ba6

@teamleaderleo
teamleaderleo merged commit d78434a into main Sep 30, 2026
64 checks passed
@teamleaderleo
teamleaderleo deleted the parity/flag-review-report branch September 30, 2026 14:39
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 087ec6dd88: every check was green at merge (17 verified; 20 skipped by policy). Full suite runs on main after merge.

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.

1 participant