feat(mcp): add billing_status, subscription_status, and is_account_locked to check_airbyte_cloud_workspace MCP tool#965
Conversation
Enhances the check_airbyte_cloud_workspace MCP tool to include billing status information in its response: - payment_status: Organization payment status (okay, grace_period, disabled, locked) - subscription_status: Organization subscription status (pre_subscription, subscribed) - can_run_syncs: Boolean indicating if syncs can run based on billing status This allows the Coral Agent to detect when syncs are disabled due to unpaid invoices and provide appropriate context to users. Co-Authored-By: AJ Steers <aj@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1769624968-add-billing-status-to-mcp' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1769624968-add-billing-status-to-mcp'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
|
The intent here is to fail gracefully - if billing info can't be fetched (e.g., due to permissions or API issues), we want to continue with I can add logging here for debuggability. The pattern already exists in this codebase (see Aaron ("AJ") Steers (@aaronsteers) - Would you like me to add logging here, or is the silent fallback acceptable for this use case? Happy to add it if you prefer the extra debuggability. |
📝 Walkthrough🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Fix bug where can_run_syncs was only computed when BOTH payment_status and subscription_status were present. Now correctly computes the value when either status is available, ensuring syncs are blocked if payment_status is 'disabled'/'locked' even when subscription_status is unavailable. Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Good catch! I've fixed this in commit 49c2e0f. The logic now correctly computes if payment_status or subscription_status:
blocked_by_payment = payment_status in {"disabled", "locked"}
blocked_by_subscription = subscription_status == "unsubscribed"
can_run_syncs = not (blocked_by_payment or blocked_by_subscription)This ensures that if |
- Add LOCKED_PAYMENT_STATUSES and LOCKED_SUBSCRIPTION_STATUSES constants - Simplify can_run_syncs logic per code review feedback - Add comment to explain billing lookup failure handling Co-Authored-By: AJ Steers <aj@airbyte.io>
|
Aaron ("AJ") Steers (@aaronsteers) Done! Simplified the logic with constants as you suggested: LOCKED_PAYMENT_STATUSES: set[str] = {"disabled", "locked"}
LOCKED_SUBSCRIPTION_STATUSES: set[str] = {"unsubscribed"}
can_run_syncs = (
payment_status
and payment_status not in LOCKED_PAYMENT_STATUSES
) or (
subscription_status
and subscription_status not in LOCKED_SUBSCRIPTION_STATUSES
)Also added a comment to the except block to address the CodeQL warning about empty except clauses. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte/mcp/cloud.py`:
- Around line 576-580: The current can_run_syncs boolean uses OR and therefore
allows syncs if either payment_status or subscription_status is non-locked;
change the logic to require both statuses to be non-blocking by using AND:
ensure can_run_syncs evaluates to True only when (payment_status is missing or
not in LOCKED_PAYMENT_STATUSES) AND (subscription_status is missing or not in
LOCKED_SUBSCRIPTION_STATUSES); update the inline comment near can_run_syncs to
reflect that both payment and subscription must permit syncs.
Update docstring to accurately describe the OR logic behavior: syncs can run if EITHER status is valid, and null status doesn't block syncs on its own. Co-Authored-By: AJ Steers <aj@airbyte.io>
- Remove try/except block - Remove nested if statement for billing check - Compute can_run_syncs unconditionally based on available status values Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte/mcp/cloud.py`:
- Around line 574-577: The computed can_run_syncs currently evaluates to False
when both payment_status and subscription_status are None, but the type allows
None to represent "unknown"; change the logic so can_run_syncs is set to None
when billing info is unavailable (both payment_status and subscription_status
are None), otherwise compute the boolean as before using the existing expression
that checks against LOCKED_PAYMENT_STATUSES and LOCKED_SUBSCRIPTION_STATUSES
(refer to the can_run_syncs variable and the payment_status/subscription_status
symbols).
Remove if block and use ternary expression with else {} as requested.
Co-Authored-By: AJ Steers <aj@airbyte.io>
Use suppress(Exception) to handle potential API failures gracefully when fetching organization info for billing status. Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
billing_status, subscription_status, and is_account_locked to check_airbyte_cloud_workspace MCP tool
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte/mcp/cloud.py`:
- Around line 171-182: Docstring for subscription_status omits the
'unsubscribed' value even though account_is_locked logic treats 'unsubscribed'
as locking; update the subscription_status docstring to include 'unsubscribed'
(e.g., "pre_subscription', 'subscribed', 'unsubscribed'") and ensure the
account_is_locked docstring references the same values consistently (mentioning
that 'unsubscribed' contributes to a locked state alongside payment_status
values like 'disabled'/'locked').
🧹 Nitpick comments (1)
airbyte/mcp/cloud.py (1)
42-44: Clarify the tuple rationale comment?The “unhashable types” note feels a bit off for string constants. Would you consider rewording it to emphasize immutability or just drop the note for clarity, wdyt?
Co-Authored-By: AJ Steers <aj@airbyte.io>
Co-Authored-By: AJ Steers <aj@airbyte.io>
…ribe_cloud_organization - Move LOCKED_PAYMENT_STATUSES and LOCKED_SUBSCRIPTION_STATUSES constants to api_util.py - Add is_account_locked() utility function to api_util.py - Add get_organization_info() function to api_util.py for direct org billing lookup - Add billing fields (payment_status, subscription_status, account_is_locked) to CloudOrganizationResult - Update describe_cloud_organization to fetch and return billing info - Update check_airbyte_cloud_workspace to use common is_account_locked() utility Co-Authored-By: AJ Steers <aj@airbyte.io>
- Convert CloudOrganization from dataclass to class with lazy loading - Add payment_status, subscription_status, account_is_locked properties - Update CloudWorkspace.get_organization() to pass credentials to CloudOrganization - Update _resolve_organization to return CloudOrganization with credentials - Update describe_cloud_organization MCP tool to use CloudOrganization properties directly Co-Authored-By: AJ Steers <aj@airbyte.io>
Aldo Gonzalez (aldogonzalez8)
left a comment
There was a problem hiding this comment.
APPROVED
- Add _organization_info_fetch_failed flag to remember when fetching fails - Return empty dict on failure instead of retrying on each property access - Simplify property implementations by removing redundant try/except blocks - Properties now return None gracefully when billing info is unavailable Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte/cloud/workspaces.py`:
- Around line 134-139: The early-return checks block re-fetching when
_organization_info_fetch_failed is True even if force_refresh is requested;
update the logic in the method that uses force_refresh,
_organization_info_fetch_failed, and _organization_info so that when
force_refresh is True you clear or reset _organization_info_fetch_failed (or run
the fetch path regardless) before returning; in short, check force_refresh first
(or set self._organization_info_fetch_failed = False when force_refresh is True)
so a forced refresh will actually attempt to re-fetch rather than immediately
returning {}.
🧹 Nitpick comments (1)
airbyte/cloud/workspaces.py (1)
149-152: Consider adding debug logging when fetch fails?The silent failure is great for graceful degradation, but it might make debugging tricky when something goes wrong. Would adding a debug-level log message help with troubleshooting, wdyt?
except Exception as e: # Consider: import logging; logger = logging.getLogger(__name__) # logger.debug("Failed to fetch organization info: %s", e) self._organization_info_fetch_failed = True return {}
When force_refresh=True is passed to _fetch_organization_info(), reset the _organization_info_fetch_failed flag so that a forced refresh will actually attempt to re-fetch rather than returning empty dict. Co-Authored-By: AJ Steers <aj@airbyte.io>
feat(mcp): add billing status to check_airbyte_cloud_workspace tool
Summary
Enhances the
check_airbyte_cloud_workspaceMCP tool to expose billing status information, allowing the Coral Agent to detect when syncs are disabled due to unpaid invoices.Adds three new fields to
CloudWorkspaceResult:payment_status: Organization payment status (e.g., 'okay', 'grace_period', 'disabled', 'locked')subscription_status: Organization subscription status (e.g., 'pre_subscription', 'subscribed')account_is_locked: Boolean indicating if the account is locked due to billing issues (defaults toFalse)The billing data is fetched from the existing
get_workspace_organization_infoAPI endpoint. Thepayment_statusandsubscription_statusfields gracefully degrade toNoneif billing info is unavailable (e.g., insufficient permissions), whileaccount_is_lockeddefaults toFalseunless we have affirmative evidence of a locked state.Updates since last revision
can_run_syncstoaccount_is_lockedwith inverted logic per code review feedbackaccount_is_lockedisTrueonly whenpayment_status in {'disabled', 'locked'}orsubscription_status in {'unsubscribed'}NonetoFalse- assumes account is not locked unless we detect a locked stateOrganizationPaymentConfig.javaenumsReview & Testing Checklist for Human
account_is_lockedlogic is correct: Should beTruewhen payment_status is 'disabled'/'locked' OR subscription_status is 'unsubscribed'. Defaults toFalsewhen billing info is unavailable.account_is_locked=Truefor a workspace with unpaid invoices (payment_status='disabled')None,account_is_lockedshould beFalse)Suggested test plan: Use the MCP tool against a test workspace with known billing status and verify the returned values match expectations.
Notes
Summary by CodeRabbit
New Features
Behavior
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.