Skip to content

Conversation

@KrshnKush
Copy link

@KrshnKush KrshnKush commented Dec 30, 2025

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

  • Closes: N/A
  • Related: N/A

2. Or, if no issue exists, describe the change:

Problem:
The CI check for forbidden cli package imports uses an overly broad regex pattern ^from.*cli.*import.*$ that incorrectly matches any import containing the substring "cli". This causes false positives for legitimate imports like:

  • from a2a.client import Client
  • from a2a.client.card_resolver import A2ACardResolver
  • from mycli import something

These are valid imports from packages named client, mycli, etc., not the forbidden cli package.
PR where issue was found: #4023

Solution:
Updated the regex pattern to ^from\s+([^ ]*\.)?cli(\s|\.) which correctly matches only imports from a package literally named cli:

  • ^from\s+ - matches "from" followed by whitespace
  • ([^ ]*\.)? - optionally matches a module path prefix (e.g., google.adk.)
  • cli - the literal "cli" package name
  • (\s|\.) - must be followed by whitespace (for import) or a dot (for submodules)

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Manual Verification:

Tested the regex patterns against various import statements:

$ cat /tmp/test_imports.txt
from a2a.client import Client as A2AClient
from a2a.client.card_resolver import A2ACardResolver
from a2a.client.client import ClientConfig as A2AClientConfig
from client import something
from cli import something
from google.adk.cli import bar
from google.adk.cli.utils import baz
from mycli import foo
from precli.something import bar

$ echo "=== NEW pattern ===" && grep -E '^from\s+([^ ]*\.)?cli(\s|\.)' /tmp/test_imports.txt
=== NEW pattern ===
from cli import something
from google.adk.cli import bar
from google.adk.cli.utils import baz

$ echo "=== OLD pattern (buggy) ===" && grep -E '^from.*cli.*import.*$' /tmp/test_imports.txt
=== OLD pattern (buggy) ===
from a2a.client import Client as A2AClient
from a2a.client.card_resolver import A2ACardResolver
from a2a.client.client import ClientConfig as A2AClientConfig
from client import something
from cli import something
from google.adk.cli import bar
from google.adk.cli.utils import baz
from mycli import foo
from precli.something import bar
Import Old Pattern New Pattern
from a2a.client import Client Matched (false positive) No match (correct)
from client import something Matched (false positive) No match (correct)
from mycli import foo Matched (false positive) No match (correct)
from precli.something import bar Matched (false positive) No match (correct)
from cli import something Matched (correct) Matched (correct)
from google.adk.cli import bar Matched (correct) Matched (correct)
from google.adk.cli.utils import baz Matched (correct) Matched (correct)

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Files changed:

  • .github/workflows/check-file-contents.yml - Fixed regex pattern for cli import detection

@gemini-code-assist
Copy link
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@adk-bot adk-bot added the tools [Component] This issue is related to tools label Dec 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants