Skip to content

fix plan gate missing plans denial - #1703

Open
erichanwang wants to merge 1 commit into
rocketride-org:developfrom
erichanwang:fix/plan-gate-missing-plans
Open

fix plan gate missing plans denial#1703
erichanwang wants to merge 1 commit into
rocketride-org:developfrom
erichanwang:fix/plan-gate-missing-plans

Conversation

@erichanwang

@erichanwang erichanwang commented Jul 28, 2026

Copy link
Copy Markdown

fixes #1672

changes:

  • treat missing account_info.plans as no entitlements instead of raising AttributeError
  • keep the ai.web.AccountInfo import type-check-only so the validator does not load the web server at runtime
  • add a regression for account objects without a plans field
  • scope the source-tree import stubs used by the focused test and restore sys.modules after importing the validator

CodeRabbit:

  • addressed the import-stub persistence thread; it is now resolved/outdated after commit 3eaa268b

verification:

  • PYTHONPATH=packages/ai/src .venv/bin/pytest packages/ai/tests/ai/common/account/test_pipeline_validation.py (12 passed; pytest warned that the local pytest install does not know the repo timeout option)
  • git diff --check
  • .venv/bin/ruff check packages/ai/src/ai/common/account/pipeline_validation.py packages/ai/tests/ai/common/account/test_pipeline_validation.py
  • .venv/bin/ruff format --check packages/ai/src/ai/common/account/pipeline_validation.py packages/ai/tests/ai/common/account/test_pipeline_validation.py
  • pre-commit hook with .venv/bin on PATH: gitleaks, ruff check, ruff format --check

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

AccountPipelineValidation now avoids a runtime AccountInfo import and safely treats missing account plans as empty. Tests isolate the module from external dependencies and verify that an account without plans is rejected.

Changes

Account plan validation

Layer / File(s) Summary
Defensive validation and deferred typing
packages/ai/src/ai/common/account/pipeline_validation.py
Moves the AccountInfo import behind TYPE_CHECKING, uses a forward-reference annotation, and defaults missing plans data to an empty collection.
Missing-plans regression coverage
packages/ai/tests/ai/common/account/test_pipeline_validation.py
Stubs external modules for isolated tests and verifies validation returns False when the account lacks plans.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jmaionchi, stepmikhaylov, rod-christensen

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code now denies missing plans cleanly, avoids the AttributeError, and adds regression coverage as requested by #1672.
Out of Scope Changes check ✅ Passed The test import stubbing and type-check-only import support the plan-gate fix and do not introduce unrelated scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is clearly related to the main change: fixing plan-gate validation when account plans are missing.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 github-actions Bot added the module:ai AI/ML modules label Jul 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor
🤖 Internal: Discord sync marker

Auto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete.

@erichanwang
erichanwang force-pushed the fix/plan-gate-missing-plans branch from f76abc8 to ad74537 Compare July 29, 2026 07:36
@erichanwang
erichanwang marked this pull request as ready for review July 29, 2026 19:49
@erichanwang

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 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 `@packages/ai/tests/ai/common/account/test_pipeline_validation.py`:
- Around line 18-25: Remove the global sys.modules stubs from
test_pipeline_validation.py and move the import-stubbing logic into a reusable
helper in packages/ai/tests/ai/modules/task/conftest.py. Ensure the helper
temporarily installs depends and rocketlib only while importing
pipeline_validation, then restores any pre-existing modules or removes the stubs
afterward so collection order and installed engines cannot affect the imported
dependency.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d9c11a9e-6cb8-43ef-9aa7-b295aeb50575

📥 Commits

Reviewing files that changed from the base of the PR and between 2cbd567 and ad74537.

📒 Files selected for processing (2)
  • packages/ai/src/ai/common/account/pipeline_validation.py
  • packages/ai/tests/ai/common/account/test_pipeline_validation.py

Comment thread packages/ai/tests/ai/common/account/test_pipeline_validation.py Outdated
@erichanwang
erichanwang force-pushed the fix/plan-gate-missing-plans branch from ad74537 to 3eaa268 Compare July 29, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ai AI/ML modules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plan gate crashes instead of denying: AccountPipelineValidation reads account_info.plans, which is not a field on AccountInfo

1 participant