Skip to content

Revert 3 commits pushed directly to main#5885

Merged
beastoin merged 3 commits intomainfrom
fix/revert-direct-main-pushes
Mar 21, 2026
Merged

Revert 3 commits pushed directly to main#5885
beastoin merged 3 commits intomainfrom
fix/revert-direct-main-pushes

Conversation

@beastoin
Copy link
Copy Markdown
Collaborator

Summary

  • Reverts 3 commits that were incorrectly pushed directly to main instead of through a PR
  • Reverted commits:
    • af8606f67 — Update test harness to match safe cast fix
    • a68365d7a — Fix safe cast for fair use stage and add refresh support
    • dd690961b — Bump app version to 784

Why

These changes should have gone through a PR. The fixes (safe cast, refresh support, evidence cleanup, version bump) will be re-submitted as a proper PR after this revert lands.

🤖 Generated with Claude Code

by AI for @beastoin

@beastoin beastoin merged commit 3d91379 into main Mar 21, 2026
3 checks passed
@beastoin beastoin deleted the fix/revert-direct-main-pushes branch March 21, 2026 13:51
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 21, 2026

Greptile Summary

This PR reverts three commits (dd690961b, a68365d7a, af8606f67) that were pushed directly to main without going through a PR, restoring the branch to the state before those changes. The reverted work (safe cast fix for fair use stage, refresh support for _loadFairUseStatus, version bump to 784) will be re-submitted as a proper PR.

Key points:

  • The revert is mostly correct and matches the intent described in the PR description.
  • The as String? ?? 'none' unsafe cast pattern is knowingly reintroduced as a temporary regression — ?? 'none' only handles null, not non-null non-String values, which would throw a TypeError. The follow-up PR is expected to address this.
  • One small impurity: a new comment (// Using Future.wait to run both fetches concurrently) is added at usage_page.dart:584 that did not exist before the reverted commits, making this a slightly non-clean revert.
  • _loadFairUseStatus() is removed from the pull-to-refresh handler as part of the revert, meaning fair use status will not update on pull-to-refresh until the follow-up PR lands.

Confidence Score: 4/5

  • Safe to merge as a cleanup revert; the known unsafe cast regression is intentional and tracked for a follow-up PR.
  • The PR achieves its stated goal of reverting three direct-to-main commits. The unsafe cast regression is acknowledged and temporary. The only unintentional change is a single explanatory comment that does not affect runtime behavior. The follow-up PR to re-land the fixes properly should be prioritized to avoid leaving the unsafe cast in production.
  • app/lib/pages/settings/usage_page.dart — contains the unsafe cast and the extra comment introduced by the revert.

Important Files Changed

Filename Overview
app/lib/pages/settings/usage_page.dart Reverts safe is String cast guard to as String? ?? 'none' (known regression, follow-up planned) and removes _loadFairUseStatus() from pull-to-refresh; also adds an extra comment not present in the original code.
app/pubspec.yaml Clean version bump revert from 784 back to 783 as expected.
app/test/widgets/usage_page_fair_use_banner_test.dart Test harness reverted to match the production cast pattern; note there is no test covering a non-null, non-String stage value, which is the scenario that would expose the unsafe cast bug reintroduced by this revert.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Pull-to-Refresh triggered] --> B["Future.wait(\n  fetchUsageStats,\n  fetchSubscription\n)"]
    B --> C[Stats & Subscription updated]
    C --> D{Fair use status\nstale?}
    D -->|Yes - not refreshed| E[Banner shows old stage]
    D -->|No| F[Banner up to date]

    subgraph "After revert (this PR)"
        B
    end

    subgraph "Reverted commit a68365d7a"
        B2["Future.wait(\n  fetchUsageStats,\n  fetchSubscription,\n  _loadFairUseStatus\n)"] --> C2[All three updated]
    end

    G[_buildFairUseBanner called] --> H["_fairUseStatus!\['stage'\]\nas String? ?? 'none'"]
    H -->|null| I[stage = 'none' → hidden]
    H -->|non-null String| J[stage = value → shown]
    H -->|non-null non-String| K["⚠️ TypeError thrown"]
Loading

Last reviewed commit: "Revert "Bump app ver..."

Comment on lines +584 to +585
// Using Future.wait to run both fetches concurrently
await Future.wait([provider.fetchUsageStats(period: period), provider.fetchSubscription()]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Extra comment not present in original code

The comment // Using Future.wait to run both fetches concurrently was not in the codebase before the three commits being reverted. This means the revert is not a clean reversal — it introduces a net-new change. A pure revert should restore the exact prior state. Consider removing this comment to keep the diff minimal and the revert verifiably clean.

Suggested change
// Using Future.wait to run both fetches concurrently
await Future.wait([provider.fetchUsageStats(period: period), provider.fetchSubscription()]);
await Future.wait([provider.fetchUsageStats(period: period), provider.fetchSubscription()]);

if (_fairUseStatus == null) return const SizedBox.shrink();
final rawStage = _fairUseStatus!['stage'];
final stage = rawStage is String ? rawStage : 'none';
final stage = _fairUseStatus!['stage'] as String? ?? 'none';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unsafe cast reintroduced by this revert

_fairUseStatus!['stage'] as String? ?? 'none' will only safely handle a null value — the ?? 'none' fallback is never reached for non-null, non-String values (e.g., an int or bool). Dart's as operator throws a TypeError at runtime when the actual type doesn't match. The reverted commit a68365d7a replaced this with the defensive rawStage is String ? rawStage : 'none' pattern specifically to guard against this.

This is noted in the PR description as a known regression that will be addressed in a follow-up PR, but it's worth flagging so reviewers are aware this is an active risk until that follow-up lands.

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
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