feat: add open, labels/tags, summary, audit, and mcp commands#34
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 28 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds governance audits, hierarchy snapshot/aggregation, console URL generation, new CLI commands (summary, audit, labels, tags, open, mcp), an optional MCP server exposing gcpath tools, TOON serializers, and extensive tests. ChangesGovernance and Metadata Inspection Foundation
Output Formatting and CLI Commands
MCP Server Integration
Test Coverage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
==========================================
+ Coverage 86.93% 87.34% +0.41%
==========================================
Files 11 13 +2
Lines 2265 3003 +738
==========================================
+ Hits 1969 2623 +654
- Misses 296 380 +84 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the gcpath tool, including a new hierarchy audit system, metadata aggregation for labels and tags, and an MCP server for AI agent integration. It also adds commands to open resources in the GCP Console and generate hierarchy summaries. Reviewers identified several logic issues, specifically regarding incorrect depth calculations for projects in the summary, cache key collisions in the MCP server due to missing metadata parameters, and inconsistent resource listing behavior when scoped. Additionally, it was recommended to refactor private metadata aggregation logic into a shared module to avoid cross-module dependencies.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
src/gcpath/core.py (1)
797-871: ⚡ Quick winRefactor
Hierarchy.summary()to reduce cognitive complexity and clear Sonar failure.Line 797 currently combines aggregation, ranking, per-org counting, and deepest-path extraction in one method, which is tripping the configured complexity gate. Split into small private helpers (e.g.,
_count_metadata_keys,_build_org_rows,_compute_deepest_paths) and keepsummary()as orchestration only.🤖 Prompt for 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. In `@src/gcpath/core.py` around lines 797 - 871, The summary method is overly complex; refactor by extracting logical blocks into small private helpers: implement _count_metadata_keys(self) to return label_counter and tag_counter computed from self.folders and self.projects, _build_org_rows(self, real_orgs) to produce org_rows using self.organizations, self.folders and self.projects, and _compute_deepest_paths(self, deepest_n) to build candidate_paths and return the deduplicated deepest_paths; then simplify summary(self, top_n=5, deepest_n=3) to call these helpers, compute top_label_keys/top_tag_keys from _count_metadata_keys, compute real_orgs and max_depth (or move max_depth into a tiny helper), and return the same dict—use the existing symbols (summary, self.folders, self.projects, self.organizations) so callers and behavior remain unchanged.src/gcpath/cli.py (2)
1409-1543: ⚖️ Poor tradeoff
audit_commandhas CC=25 (SonarCloud CI failure)The function conflates input validation, scope resolution, hierarchy loading, audit execution, and three output-format renderers. Extracting the Rich render block into
_audit_rich(issues, sev_counts)(mirroring_stats_rich,_cache_status_rich, etc.) would reduce complexity and make the pattern consistent with the rest of the file.🤖 Prompt for 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. In `@src/gcpath/cli.py` around lines 1409 - 1543, The audit_command function is too large; move the rich-format rendering block into a new helper function named _audit_rich(issues, sev_counts) (matching patterns like _stats_rich/_cache_status_rich). Implement _audit_rich to import rich.table.Table and rich.console.Console, build the same table (columns: Severity, Check, Path, Type, Details), color-map severities, and print via Console; then replace the current inline "elif fmt == 'rich':" block in audit_command with a single call to _audit_rich(issues, sev_counts). Ensure _audit_rich handles the empty-issues case the same way (rprint("[green]No audit issues found.[/green]")) and keep all existing behavior and variable names.
1603-1623: ⚡ Quick win
_resolve_path_to_objectduplicatesmcp_server._resolve_to_objectand has a SonarCloud CC=18 failureBoth functions iterate
hierarchy.organizations/hierarchy.projectsto look up a resource by name. The only difference is that the CLI version first callshierarchy.get_resource_name(path)before the lookup. A single shared helper incore.py(or onHierarchy) covering the lookup-by-name step would eliminate both the duplication and reduce complexity in each call site.🤖 Prompt for 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. In `@src/gcpath/cli.py` around lines 1603 - 1623, The _resolve_path_to_object function duplicates logic in mcp_server._resolve_to_object by iterating hierarchy.organizations and hierarchy.projects to find a resource by name; extract that lookup-by-name logic into a single shared helper (either a new method on Hierarchy, e.g., Hierarchy.find_resource_by_name(name) or a core.py function) that accepts the resource name and returns the matching OrganizationNode/Folder/Project or raises GCPathError, then update _resolve_path_to_object to call hierarchy.get_resource_name(path) and delegate the lookup to the new helper and similarly update mcp_server._resolve_to_object to use the same helper to remove duplication and lower complexity.src/gcpath/mcp_server.py (1)
108-277: 🏗️ Heavy lift
build_servercognitive complexity is 64 — a SonarCloud CI failure (limit: 15)All 9 tool functions are defined as closures inside
build_server, which inflates its measured complexity. Extracting each tool into a standalone module-level function (acceptinghierarchy_loader: Callableor a thin context object) and registering them separately would fix the CI check and make the tools independently testable.🤖 Prompt for 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. In `@src/gcpath/mcp_server.py` around lines 108 - 277, The build_server function contains nine nested tool closures (path_to_name, name_to_path, list_resources, find_resources, get_ancestors, get_summary, get_labels, get_tags, get_console_url, audit_hierarchy) which spikes its cognitive complexity; extract each tool into its own top-level function that accepts a hierarchy loader or thin context (e.g. a parameter that calls _get_hierarchy or provides use_asset_api and scope) and returns the same behavior, then in build_server simply import/register those functions with server.tool() (or wrap them to bind the loader) so build_server only constructs FastMCP and registers pre-defined module-level handlers, reducing complexity and making each tool testable.
🤖 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 `@src/gcpath/audit.py`:
- Around line 47-50: The audit path builder _resource_path currently formats
organization names as f"//{item.organization.display_name}" without escaping;
update _resource_path so that item.organization.display_name is passed through
the project's canonical path-escaping utility (the same function used elsewhere
for gcpath paths) before formatting, e.g., replace direct display_name usage
with the escaped display name when building the "//{...}" string so names with
spaces/special chars produce canonical gcpath-style paths.
In `@src/gcpath/cli.py`:
- Around line 141-175: Move the pure data-aggregation helper _aggregate_metadata
out of cli.py into core.py: copy the entire function (including its docstring)
into core.py, add the necessary typing and domain imports there (List, Union,
Tuple, Dict, Any, Optional and Folder, Project), and remove the
_aggregate_metadata definition from cli.py; then update any callers (replace
from gcpath.cli import _aggregate_metadata with an import from the core module)
such as in mcp_server.py and cli.py so they import _aggregate_metadata from core
instead, preserving the function name and behavior.
- Around line 2108-2115: Remove the misleading try/except around "from
gcpath.mcp_server import run_server" — import run_server directly instead of
catching ImportError that can’t signal a missing "mcp" dependency; delete the
except ImportError block and the toon_error/typer.Exit arms so build_server’s
GCPathError continues to surface as intended (look for the import of run_server
in src/gcpath/cli.py and remove the surrounding try/except).
- Around line 1627-1715: open_resource is overly complex (CC=44) and currently
drops per-item errors when --browser opens at least one URL; extract the
browser-open loop and the output rendering into two helpers (e.g.
_open_results_in_browser(results, ctx) and _render_open_results(results, ctx,
help_lines)) to reduce cognitive complexity, and modify the browser helper so
that after attempting to open each URL it always surfaces any non-URL/error rows
to the user (use existing toon_open/rprint/json/yaml dumper logic from the main
flow) — specifically ensure that when opened > 0 you still call the rendering
helper for rows with error entries (or print them to stderr/rich) instead of
returning early; update open_resource to call these helpers.
In `@src/gcpath/formatters.py`:
- Around line 47-53: The console_url() handling for Project currently only
rejects organizationless projects but allows projects tied to the synthetic org;
update the Project branch in console_url() to detect and reject synthetic-org
projects by checking item.organization (and if item.folder is present, the
folder's organization) against the SYNTHETIC_ORG_NAME constant and raise
GCPathError (same style as existing checks) instead of returning a console URL
when the org is synthetic. Ensure you reference the SYNTHETIC_ORG_NAME constant,
the Project type, the console_url() function, GCPathError, and the
item.organization / item.folder attributes when making the change.
In `@src/gcpath/mcp_server.py`:
- Around line 13-14: mcp_server.py currently imports the private helper
_aggregate_metadata from gcpath.cli, creating an upward dependency; move the
_aggregate_metadata function into core.py (or a new aggregators.py) as a public
function, update its module-level docstring and tests if needed, then change
mcp_server.py to import the relocated symbol (e.g., from gcpath.core import
aggregate_metadata or from gcpath.aggregators import aggregate_metadata) and
remove the gcpath.cli import; ensure gcpath.cli is updated to import the same
new location so both CLI and MCP use the shared core aggregator without
introducing cross-layer imports.
- Around line 151-170: The list_resources function is adding all projects when
resource_scope is None (elif not resource_scope) causing nested projects to
appear while folders are limited to root-level; in the projects loop (for p in
h.projects) change the branch so that when no resource_scope and not recursive
you only append projects whose parent starts with "organizations/" (mirror the
folders logic using p.parent.startswith("organizations/")) so root-level
behavior is consistent with folders.
In `@tests/test_audit.py`:
- Around line 105-108: test_audit_severity_filter is vacuously passing because
make_test_hierarchy() yields no "error" level issues so filtering by
severity="error" returns an empty list; change the test to ensure the input
produces at least one error-level issue before asserting all severities: either
construct/modify the test fixture (use make_test_hierarchy or a new helper) to
include a node that triggers an "error" severity, or call run_audit against a
different test hierarchy that contains an error, then assert both that the
returned issues list is non-empty and that all(i["severity"] == "error" for i in
issues); reference the test function test_audit_severity_filter, the fixture
creator make_test_hierarchy, and the audited function run_audit when making the
change.
In `@tests/test_serializers.py`:
- Around line 534-555: Test asserts the grammatically incorrect "1 issues";
update the implementation and test for correct pluralisation: in toon_audit
ensure the summary uses "issue" when total == 1 and "issues" otherwise (adjust
the code path that builds the summary string), and update
TestToonAudit.test_audit_with_issues to assert for the singular form ("1 issue")
instead of "1 issues" so the test matches the corrected toon_audit behavior.
---
Nitpick comments:
In `@src/gcpath/cli.py`:
- Around line 1409-1543: The audit_command function is too large; move the
rich-format rendering block into a new helper function named _audit_rich(issues,
sev_counts) (matching patterns like _stats_rich/_cache_status_rich). Implement
_audit_rich to import rich.table.Table and rich.console.Console, build the same
table (columns: Severity, Check, Path, Type, Details), color-map severities, and
print via Console; then replace the current inline "elif fmt == 'rich':" block
in audit_command with a single call to _audit_rich(issues, sev_counts). Ensure
_audit_rich handles the empty-issues case the same way (rprint("[green]No audit
issues found.[/green]")) and keep all existing behavior and variable names.
- Around line 1603-1623: The _resolve_path_to_object function duplicates logic
in mcp_server._resolve_to_object by iterating hierarchy.organizations and
hierarchy.projects to find a resource by name; extract that lookup-by-name logic
into a single shared helper (either a new method on Hierarchy, e.g.,
Hierarchy.find_resource_by_name(name) or a core.py function) that accepts the
resource name and returns the matching OrganizationNode/Folder/Project or raises
GCPathError, then update _resolve_path_to_object to call
hierarchy.get_resource_name(path) and delegate the lookup to the new helper and
similarly update mcp_server._resolve_to_object to use the same helper to remove
duplication and lower complexity.
In `@src/gcpath/core.py`:
- Around line 797-871: The summary method is overly complex; refactor by
extracting logical blocks into small private helpers: implement
_count_metadata_keys(self) to return label_counter and tag_counter computed from
self.folders and self.projects, _build_org_rows(self, real_orgs) to produce
org_rows using self.organizations, self.folders and self.projects, and
_compute_deepest_paths(self, deepest_n) to build candidate_paths and return the
deduplicated deepest_paths; then simplify summary(self, top_n=5, deepest_n=3) to
call these helpers, compute top_label_keys/top_tag_keys from
_count_metadata_keys, compute real_orgs and max_depth (or move max_depth into a
tiny helper), and return the same dict—use the existing symbols (summary,
self.folders, self.projects, self.organizations) so callers and behavior remain
unchanged.
In `@src/gcpath/mcp_server.py`:
- Around line 108-277: The build_server function contains nine nested tool
closures (path_to_name, name_to_path, list_resources, find_resources,
get_ancestors, get_summary, get_labels, get_tags, get_console_url,
audit_hierarchy) which spikes its cognitive complexity; extract each tool into
its own top-level function that accepts a hierarchy loader or thin context (e.g.
a parameter that calls _get_hierarchy or provides use_asset_api and scope) and
returns the same behavior, then in build_server simply import/register those
functions with server.tool() (or wrap them to bind the loader) so build_server
only constructs FastMCP and registers pre-defined module-level handlers,
reducing complexity and making each tool testable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c043b34b-2f97-45b3-99bc-8da103063111
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
pyproject.tomlsrc/gcpath/audit.pysrc/gcpath/cli.pysrc/gcpath/core.pysrc/gcpath/formatters.pysrc/gcpath/mcp_server.pysrc/gcpath/serializers.pytests/test_audit.pytests/test_cli.pytests/test_core.pytests/test_formatters.pytests/test_serializers.py
tardigrde
commented
May 8, 2026
tardigrde
left a comment
tardigrde
left a comment
Owner
Author
There was a problem hiding this comment.
Adversarial Review
This PR adds five substantial features (open, labels, tags, summary, audit, mcp). The code is generally clean and the tests cover the happy paths. However there are several correctness bugs, one logic error in the MCP layer, a silent-skip usability trap in audit, and meaningful test coverage gaps.
HIGH — fix before merge:
core.py:summary()max_depthis computed from folders only — projects at deeper levels are ignored, makingmax_depthsystematically wrong.core.py:summary()project depth formula indeepest_pathsuseslen(folder.ancestors)+1but should uselen(folder.ancestors)— off by two relative to the folder depth formula.mcp_server.py:list_resourceswith no scope andrecursive=Falseleaks ALL projects (including deeply-nested ones), not just org-level resources.audit.py:_check_synthetic_orgonly reports folders under the synthetic org, silently ignoring projects.audit.py:run_auditsilently skipsmissing_required_label/name_pattern_violationwhen companion arg is absent, showing a falsely clean0 issues.
MEDIUM:
6. cli.py:open_resource browser mode silently discards GCPathError rows when at least one URL opened successfully.
7. mcp_server.py:_get_hierarchy cache key ignores include_labels/include_tags — stale labelless cache silently serves callers that need labels.
8. mcp_server.py imports _aggregate_metadata from gcpath.cli at module top level — loads full Typer app on every import gcpath.mcp_server.
9. mcp_server.py:audit_hierarchy exposes raw regex name_pattern to MCP clients with no timeout — ReDoS risk.
10. toon_metadata_aggregation emits two TOON blocks; toon_ls emits one. TOON-parsing agents see them differently.
LOW / NITPICK:
11. No tests/test_mcp_server.py — MCP tool logic has zero unit tests.
12. mcp_server.py:name_to_path makes live GCP API calls instead of using the cached in-memory hierarchy.
13. test_mcp_missing_dependency_emits_help asserts exit_code in (0, 1) — can never fail.
tardigrde
added a commit
that referenced
this pull request
May 8, 2026
Reviews (high-severity correctness):
- core.summary: include projects in max_depth and use the correct
project depth formula (folder.depth + 1 == len(folder.ancestors)) so
deepest_paths and max_depth no longer underreport hierarchy depth.
- audit._check_synthetic_org: also flag projects directly attached to
the synthetic org instead of only its folders.
- formatters.console_url: reject projects that live under the synthetic
org (directly or via folder), matching the org/folder rejection.
- mcp_server.list_resources: at root scope, only return org-level
projects (mirroring the folder filter) and merge identical branches.
Reviews (mediums):
- Move _aggregate_metadata into core.aggregate_metadata so cli.py and
mcp_server.py both consume it without an upward dependency.
- mcp_server cache key now varies on include_labels/include_tags so
different metadata requirements don't reuse a labelless hierarchy.
- mcp_server.name_to_path serves resolutions from the cached hierarchy
and only falls back to live GCP calls for unknown resources.
- Reject oversized name_pattern (>200 chars) in the audit CLI and the
audit_hierarchy MCP tool to bound ReDoS risk.
- audit CLI: --check missing_required_label/name_pattern_violation now
errors out when --require-labels / --name-pattern is missing instead
of silently emitting `0 issues`.
- open --browser: surface error rows even when at least one URL opened
successfully, so partial-success no longer swallows failures.
- audit._resource_path: url-escape org display names so paths stay
canonical for orgs containing spaces/specials.
- toon_audit: pluralize correctly ("1 issue", "N issues") and emit the
count + table in a single TOON block (same fix for
toon_metadata_aggregation).
- cli.mcp_command: drop the dead ImportError wrapper around
`from gcpath.mcp_server import run_server` (mcp_server is import-safe;
the missing-extra signal already flows through GCPathError).
SonarCloud:
- Define _RICH_HEADER_STYLE / _SCOPE_ALL_ORGS constants for the
duplicated "bold magenta" / "all organizations" literals (S1192).
- Refactor open_resource (CC=44 -> below threshold) into resolve /
browser-open / render helpers, and audit_command (CC=25) into
validators + render helpers; reduce summary_command, mcp build_server,
_resolve_to_object, _resolve_path_to_object cognitive complexity by
extracting tool/render/finder helpers (S3776).
- Merge the duplicate `_serialize_resource(...)` branches in
list_resources (S1871).
CodeQL:
- tests/test_open_single_folder: assert against the fully-formed URL
rather than the host substring so the URL-substring sanitization rule
no longer flags the assertion.
- Replace tautology `assert result.exit_code in (0, 1)` with a precise
assertion against the missing-extra error path.
Codecov:
- New tests/test_mcp_server.py covers cache keying, list_resources
scoping, find_resources, name_to_path/path_to_name, console_url,
aggregation, audit_hierarchy, ReDoS guard, synthetic-org flagging,
build_server import-error path, and a smoke roundtrip through every
registered FastMCP tool.
- Additional tests for audit pattern errors, audit CLI validation,
open --browser partial-success, summary/audit rich rendering, and
the formatters.console_url synthetic-org guard.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tardigrde
added a commit
that referenced
this pull request
May 8, 2026
- formatters.console_url: replace `elif item.organization is not None` (always-true after the early-return guard) with an `else` + assertion so S2589 stops firing. - mcp_server: introduce `_PREFIX_ORGS/_FOLDERS/_PROJECTS` constants and reuse them in `_resolve_to_object` and `_include_in_listing` to clear the S1192 duplicate-literal finding. - core.Hierarchy.summary: extract `_summary_max_depth`, `_summary_deepest_paths`, and a `_project_depth` helper to drop the function below the S3776 cognitive-complexity threshold. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/gcpath/mcp_server.py (1)
197-210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRoot listing drops organizationless projects
With
effective_scope=Noneandrecursive=False, Line 210 only keeps parents starting withorganizations/. Organizationless projects are therefore omitted fromlist_resources()root output.This is the same root-level filtering area discussed earlier and still needs the orgless branch accounted for.
🐛 Proposed fix
- for p in hierarchy.projects: - if _include_in_listing(p.parent, effective_scope, recursive): - results.append(_serialize_resource(p)) + for p in hierarchy.projects: + if recursive or (effective_scope and p.parent == effective_scope): + results.append(_serialize_resource(p)) + elif not effective_scope and ( + p.parent.startswith("organizations/") or p.organization is None + ): + results.append(_serialize_resource(p))🤖 Prompt for 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. In `@src/gcpath/mcp_server.py` around lines 197 - 210, The helper _include_in_listing currently omits organizationless projects when effective_scope is None and recursive is False; update _include_in_listing to treat orgless parents as included at root by returning True when parent represents an orgless project (e.g., parent == "" or whatever sentinel your project objects use for no organization), so change the final return to include parent.startswith("organizations/") OR the orgless sentinel (refer to the _include_in_listing function and the callers in list_resources()/hierarchy.projects).
🤖 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 `@src/gcpath/core.py`:
- Around line 852-909: The summary() currently computes counts, depths, metadata
counters, and deepest_paths from all self.folders/self.projects; change it to
first build filtered lists real_folders and real_projects that exclude anything
under the synthetic org (use SYNTHETIC_ORG_NAME) by determining each resource's
owning organization (for Folder use f.organization.name if available; for
Project check p.organization.name else p.folder.organization.name), then use
real_folders/real_projects for folder_count, project_count, max_depth loops, for
computing label_counter/tag_counter (or update _summary_metadata_counters to
accept these filtered lists), and for candidate_paths/deepest_paths generation;
keep _summary_org_rows(real_orgs) as-is.
In `@src/gcpath/mcp_server.py`:
- Around line 286-290: The build_server function lacks a return type annotation;
add an explicit return type to its signature (e.g., -> FastAPI or the actual
server type returned) to satisfy mypy and project typing rules. Locate the
build_server definition and update its signature to include the correct return
annotation matching the object constructed/returned within build_server
(reference the build_server function and any factory/constructor it calls to
determine the precise return type). Ensure imports include the annotated type if
needed.
- Around line 105-112: The MCP responses currently emit organization paths with
raw display names; update _serialize_resource (the OrganizationNode branch) and
the similar branch around the org serialization later to produce canonical
escaped paths by using the existing name-to-path helper (e.g., call
name_to_path(item.organization.display_name) or, if no helper is available,
urllib.parse.quote on display_name) instead of
f"//{item.organization.display_name}"; ensure the emitted "path" value is the
escaped/canonical form (e.g., "//Example%20Org") so name_to_path and path
round-tripping remain consistent.
- Around line 244-245: The current truthy check ignores top=0; change the guard
so it only applies when top is explicitly provided (e.g., check "top is not
None" instead of "if top") before slicing rows; update the conditional around
rows = rows[:top] in mcp_server.py (the block using the top variable that slices
rows) so top=0 returns an empty list while None still means no limit.
---
Duplicate comments:
In `@src/gcpath/mcp_server.py`:
- Around line 197-210: The helper _include_in_listing currently omits
organizationless projects when effective_scope is None and recursive is False;
update _include_in_listing to treat orgless parents as included at root by
returning True when parent represents an orgless project (e.g., parent == "" or
whatever sentinel your project objects use for no organization), so change the
final return to include parent.startswith("organizations/") OR the orgless
sentinel (refer to the _include_in_listing function and the callers in
list_resources()/hierarchy.projects).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b15c7c36-7211-4cc8-8332-8bd33c9b90f7
📒 Files selected for processing (11)
src/gcpath/audit.pysrc/gcpath/cli.pysrc/gcpath/core.pysrc/gcpath/formatters.pysrc/gcpath/mcp_server.pysrc/gcpath/serializers.pytests/test_audit.pytests/test_cli.pytests/test_core.pytests/test_mcp_server.pytests/test_serializers.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/test_serializers.py
- tests/test_cli.py
- src/gcpath/serializers.py
- src/gcpath/audit.py
- src/gcpath/cli.py
tardigrde
added a commit
that referenced
this pull request
May 8, 2026
- core.summary: filter folders/projects to drop synthetic-org descendants before computing folder_count, project_count, max_depth, top label/tag keys, and deepest_paths so the snapshot isn't skewed by the artificial folder-root org. Orgless projects (no org and no folder) remain in the count — they're real GCP resources that just sit outside any visible organization. - mcp_server: url-escape organization display names in _serialize_resource and _name_to_path so paths emitted by MCP tools are canonical and round-trip cleanly through path_to_name. - mcp_server._aggregate_impl: treat `top` with `is not None` so an explicit `top=0` returns an empty list instead of being silently collapsed to "no limit". - mcp_server.build_server: add `-> "FastMCP"` return annotation (under TYPE_CHECKING import) for full mypy coverage. New tests cover: synthetic-org descendants excluded from summary metrics, MCP org-path escaping (serialize_resource + name_to_path), and the `top=0` aggregation edge case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Adds the top 5 features from the implementation plan to make gcpath stickier for daily human use and richer for AI agents. - `open <path>` opens or prints the GCP Cloud Console URL for a resource. Multi-arg supported; rejects orgless and synthetic-org resources with structured errors. - `labels` / `tags` aggregate label/tag occurrences across the hierarchy with counts, examples, and `--key`/`--top` filters. - `summary` prints a one-shot agent-friendly snapshot (counts, depth, top label/tag keys, deepest paths) and excludes the synthetic org. - `audit` runs governance checks (orphan projects, synthetic-org, required labels, duplicate display names, name-pattern violations) with severity gating and a `--exit-zero` CI flag. - `mcp` runs gcpath as an MCP server (FastMCP, stdio/sse transports) exposing path/name/list/find/ancestors/summary/labels/tags/console_url/ audit tools so Claude Desktop, Claude Code, and other MCP clients can query the hierarchy directly. Optional `[mcp]` extra in pyproject.toml. Test coverage: 420 tests passing, ruff and mypy clean.
Reviews (high-severity correctness):
- core.summary: include projects in max_depth and use the correct
project depth formula (folder.depth + 1 == len(folder.ancestors)) so
deepest_paths and max_depth no longer underreport hierarchy depth.
- audit._check_synthetic_org: also flag projects directly attached to
the synthetic org instead of only its folders.
- formatters.console_url: reject projects that live under the synthetic
org (directly or via folder), matching the org/folder rejection.
- mcp_server.list_resources: at root scope, only return org-level
projects (mirroring the folder filter) and merge identical branches.
Reviews (mediums):
- Move _aggregate_metadata into core.aggregate_metadata so cli.py and
mcp_server.py both consume it without an upward dependency.
- mcp_server cache key now varies on include_labels/include_tags so
different metadata requirements don't reuse a labelless hierarchy.
- mcp_server.name_to_path serves resolutions from the cached hierarchy
and only falls back to live GCP calls for unknown resources.
- Reject oversized name_pattern (>200 chars) in the audit CLI and the
audit_hierarchy MCP tool to bound ReDoS risk.
- audit CLI: --check missing_required_label/name_pattern_violation now
errors out when --require-labels / --name-pattern is missing instead
of silently emitting `0 issues`.
- open --browser: surface error rows even when at least one URL opened
successfully, so partial-success no longer swallows failures.
- audit._resource_path: url-escape org display names so paths stay
canonical for orgs containing spaces/specials.
- toon_audit: pluralize correctly ("1 issue", "N issues") and emit the
count + table in a single TOON block (same fix for
toon_metadata_aggregation).
- cli.mcp_command: drop the dead ImportError wrapper around
`from gcpath.mcp_server import run_server` (mcp_server is import-safe;
the missing-extra signal already flows through GCPathError).
SonarCloud:
- Define _RICH_HEADER_STYLE / _SCOPE_ALL_ORGS constants for the
duplicated "bold magenta" / "all organizations" literals (S1192).
- Refactor open_resource (CC=44 -> below threshold) into resolve /
browser-open / render helpers, and audit_command (CC=25) into
validators + render helpers; reduce summary_command, mcp build_server,
_resolve_to_object, _resolve_path_to_object cognitive complexity by
extracting tool/render/finder helpers (S3776).
- Merge the duplicate `_serialize_resource(...)` branches in
list_resources (S1871).
CodeQL:
- tests/test_open_single_folder: assert against the fully-formed URL
rather than the host substring so the URL-substring sanitization rule
no longer flags the assertion.
- Replace tautology `assert result.exit_code in (0, 1)` with a precise
assertion against the missing-extra error path.
Codecov:
- New tests/test_mcp_server.py covers cache keying, list_resources
scoping, find_resources, name_to_path/path_to_name, console_url,
aggregation, audit_hierarchy, ReDoS guard, synthetic-org flagging,
build_server import-error path, and a smoke roundtrip through every
registered FastMCP tool.
- Additional tests for audit pattern errors, audit CLI validation,
open --browser partial-success, summary/audit rich rendering, and
the formatters.console_url synthetic-org guard.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- formatters.console_url: replace `elif item.organization is not None` (always-true after the early-return guard) with an `else` + assertion so S2589 stops firing. - mcp_server: introduce `_PREFIX_ORGS/_FOLDERS/_PROJECTS` constants and reuse them in `_resolve_to_object` and `_include_in_listing` to clear the S1192 duplicate-literal finding. - core.Hierarchy.summary: extract `_summary_max_depth`, `_summary_deepest_paths`, and a `_project_depth` helper to drop the function below the S3776 cognitive-complexity threshold. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- core.summary: filter folders/projects to drop synthetic-org descendants before computing folder_count, project_count, max_depth, top label/tag keys, and deepest_paths so the snapshot isn't skewed by the artificial folder-root org. Orgless projects (no org and no folder) remain in the count — they're real GCP resources that just sit outside any visible organization. - mcp_server: url-escape organization display names in _serialize_resource and _name_to_path so paths emitted by MCP tools are canonical and round-trip cleanly through path_to_name. - mcp_server._aggregate_impl: treat `top` with `is not None` so an explicit `top=0` returns an empty list instead of being silently collapsed to "no limit". - mcp_server.build_server: add `-> "FastMCP"` return annotation (under TYPE_CHECKING import) for full mypy coverage. New tests cover: synthetic-org descendants excluded from summary metrics, MCP org-path escaping (serialize_resource + name_to_path), and the `top=0` aggregation edge case. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tardigrde
force-pushed
the
claude/implement-top-features-Sr8Px
branch
from
b2a8c08 to
b6caf24
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the top 5 features from the implementation plan to make gcpath
stickier for daily human use and richer for AI agents.
open <path>opens or prints the GCP Cloud Console URL for aresource. Multi-arg supported; rejects orgless and synthetic-org
resources with structured errors.
labels/tagsaggregate label/tag occurrences across thehierarchy with counts, examples, and
--key/--topfilters.summaryprints a one-shot agent-friendly snapshot (counts, depth,top label/tag keys, deepest paths) and excludes the synthetic org.
auditruns governance checks (orphan projects, synthetic-org,required labels, duplicate display names, name-pattern violations)
with severity gating and a
--exit-zeroCI flag.mcpruns gcpath as an MCP server (FastMCP, stdio/sse transports)exposing path/name/list/find/ancestors/summary/labels/tags/console_url/
audit tools so Claude Desktop, Claude Code, and other MCP clients can
query the hierarchy directly. Optional
[mcp]extra in pyproject.toml.Test coverage: 420 tests passing, ruff and mypy clean.
Summary by CodeRabbit
New Features
Chores