Skip to content

docs(seo): SEO audit 2026-09-13 + Search Console tools on the gcp-monitor connector [KAN-270] - #3497

Open
adamtasteslikegood wants to merge 71 commits into
devfrom
feat/kan-270-seo-audit-gsc-mcp
Open

adamtasteslikegood wants to merge 71 commits into
devfrom
feat/kan-270-seo-audit-gsc-mcp

Conversation

@adamtasteslikegood

@adamtasteslikegood adamtasteslikegood commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Summary

Last day of Adam's Semrush trial: audit what the site is missing against the SEO strategy on record, list the keyword targets, say what to fix first and how to earn visibility, and build ongoing monitoring from Google Search Console.

  • docs/seo/SEO_AUDIT_2026-09-13.md — health score 48/100 (technical 87.5; content, images, schema are the gaps). Strategy-vs-reality table across the v0.2 roadmap, the Copilot SEO plan, the canonical rollout and Sprint 10; findings in Issue/Impact/Evidence/Fix form, each tied to a follow-up ticket (KAN-271 → KAN-277); keyword families from Google autocomplete with estimates tagged; competitor snapshot; off-site plan. Shareable version: https://claude.ai/code/artifact/fab441da-5a53-4e75-8aad-7de865e87b45
  • scripts/monitoring/gsc_tools.py — eight read-only Search Console tools (gsc_sites, gsc_search_performance, gsc_compare_periods, gsc_striking_distance, gsc_sitemaps, gsc_inspect_url, gsc_index_coverage_sample, gsc_weekly_report) registered on the existing gcp-monitor FastMCP server, so they reach the already-registered Claude connector on the next redeploy. Same credential resolution as the monitoring tools; every access failure returns the exact instruction (service-account email to add in Search Console, or the API-enable command with the project number) instead of a traceback.
  • Dockerfile copies the module; google-auth/requests pinned with ceilings per the requirements rule; deploy_mcp_cloud_run.sh enables searchconsole.googleapis.com, sets GSC_SITE_URL, prints the one manual Search Console step.
  • /seo-weekly-check skill (.claude/skills/seo-weekly-check/SKILL.md), the Search Console counterpart of /system-health-check.
  • docs/MCP_GCP_MONITORING.md § 6.5 and the docs index.

What was and was not measured

  • Semrush MCP: account out of API units on every research tool — no Semrush data in this audit.
  • PageSpeed Insights anonymous quota exhausted → Lighthouse 13.4 run locally (mobile, simulated throttling) against production.
  • Search Console API: workstation gcloud needs an interactive re-login and the local key's project has the API disabled, so the new tools have not returned real data yet.

Verification

  • python3 -m unittest scripts/monitoring/test_gsc_tools.py — 66 tests pass (fake session, no network); the required PR gate now runs this suite and builds the monitoring MCP image.
  • The real FastMCP server registers all 11 tools (3 monitoring + 8 Search Console).
  • Live probe with the local key reached Google's API and returned the actionable "API not enabled in project 746675616486" message — the error path works end to end.
  • npm run format:check clean.

Pending Adam (≈15 minutes) before the tools return data

  1. gcloud auth login, then scripts/monitoring/deploy_mcp_cloud_run.sh (enables the API, prints the SA email). For the local stdio server, also enable the API in the key's project.
  2. Search Console → tasteslikegood.org → Settings → Users and permissions → add that SA email, permission Restricted. Then call gsc_sites.

Side effect of merging to dev: the Railway-hosted gcp-monitor instance rebuilds on every dev push (empty watchPatterns). Its base64-key service account will hit the same "enable the API / add the user" path and degrade cleanly — the gsc_* tools return the instruction, the monitoring tools are unaffected.

No production or template changes in this PR; the fixes are proposed with exact diffs in the report and filed as KAN-271..277.

🤖 Generated with Claude Code

https://claude.ai/code/session_017wNprJqg3APNhz8NbAD5ru


Rovo Dev code review: Out of Rovo Dev credits
You've used all your Rovo Dev credits, so Rovo Dev can't review your pull requests.

adamtasteslikegood and others added 2 commits September 13, 2026 10:34
…or [KAN-270]

Eight read-only gsc_* tools (sites, search performance, period comparison,
striking distance, sitemaps vs live sitemap, URL inspection, coverage sample,
weekly report) registered on the existing FastMCP server so they reach the
already-registered Claude connector on the next redeploy. Same credential
resolution as the monitoring tools; every access failure returns the exact
instruction (service-account email to add under Search Console → Users and
permissions, or the API-enable command with the project number) instead of a
traceback.

- Dockerfile copies gsc_tools.py next to the server; google-auth/requests
  pinned with ceilings per the requirements.txt rule.
- deploy_mcp_cloud_run.sh enables searchconsole.googleapis.com, sets
  GSC_SITE_URL, and prints the one manual Search Console step.
- /seo-weekly-check skill mirrors /system-health-check.
- 27 unit tests with a fake session; live probe reached the API and returned
  the API-disabled instruction for the local key's project.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wNprJqg3APNhz8NbAD5ru
…ords, monitoring [KAN-270]

Health score 48/100 (publisher profile). Technical 87.5; content, images and
schema are the gaps. Strategy-vs-reality table across the v0.2 roadmap, the
Copilot SEO plan, the canonical rollout and Sprint 10; findings in
Issue/Impact/Evidence/Fix form each tied to a follow-up ticket
(KAN-271..277); keyword families from Google autocomplete with estimates
tagged; competitor snapshot; off-site plan; the Search Console monitoring
built in this branch and the two setup steps it still needs.

Semrush: account out of API units, no data pulled. PageSpeed API quota
exhausted; Lighthouse run locally instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wNprJqg3APNhz8NbAD5ru
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@ai-document-creator

Copy link
Copy Markdown

Tier Estimate Files

📧 Email-friendly summary
Tier: Free | Files: 4 | Estimate: ~37,200 tokens

📊 Documentation Processing Estimate

Processing this PR will analyze and generate documentation for the changed files.

Cost Estimate

Files to Process: 4
Estimated Tokens: ~37,200 tokens
Your Tier: Free


✅ Approval Required

To proceed with documentation generation, comment:

/docbot-approve

To cancel, simply close this PR or leave it unprocessed.


Powered by Woden DocBot

Copilot AI lite review requested due to automatic review settings September 13, 2026 17:44
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
pip/google-auth >= 2.22.0,< 3.0.0 UnknownUnknown
pip/requests >= 2.31.0,< 3.0.0 UnknownUnknown

Scanned Files

  • scripts/monitoring/requirements.txt

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Junie is failed!

Details: ❌ Junie execution failed.

Errors reported by Junie:
• Junie: Insufficient account balance. All tokens in your account have been spent.

What it means:
Junie ran out of AI credits: all tokens on the balance of the account behind junie_api_key are spent.
Top up the balance or upgrade the JetBrains AI subscription, or pass your own model API key (for example anthropic_api_key / openai_api_key), and re-run the workflow.

Junie CLI exited with code 1.
Check the Junie execution logs above for more details.

View job run

…ts its own instruction [KAN-270]

The hub-page evidence quoted tag counts that no tool call had computed.
Recomputed over all 96 public pages (exact-tag counts): comfort food 23,
dinner 23, gluten-free 20, dessert 16, breakfast 14, lunch 14, sandwich 11,
pasta 9, mexican 7, tacos 6, high protein 5.

classify_http_error: a 403 carrying ACCESS_TOKEN_SCOPE_INSUFFICIENT means the
token lacks webmasters.readonly (the Cloud Run metadata path), not a missing
Search Console user — sending the operator to the user list would not help.
Third branch + test.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wNprJqg3APNhz8NbAD5ru

Copilot AI 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.

🟡 Changes recommended

The Search Console query schema defect blocks core reporting, with additional unresolved accuracy, validation, credential, error-state, and test issues.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an SEO audit and eight read-only Google Search Console tools to the existing GCP MCP connector.

Changes:

  • Adds GSC client tools, tests, and reporting helpers.
  • Updates dependencies, Docker, and Cloud Run deployment.
  • Adds SEO audit documentation and weekly-check workflow.
File summaries
File Summary
scripts/monitoring/test_gsc_tools.py Tests GSC behavior; one fixture relies on a hard-coded date and will eventually fail.
scripts/monitoring/requirements.txt Adds pinned GSC dependencies.
scripts/monitoring/gsc_tools.py Adds GSC tools. Critical: uses type instead of searchType, breaking four query tools. Moderate issues include sitemap-count labeling, swallowed fetch failures, incomplete credential errors, missing runtime service-account email, weak which/sort_by validation, and inaccurate non-positive day reporting.
scripts/monitoring/gcp_mcp_server.py Registers the GSC tools with the MCP server.
scripts/monitoring/Dockerfile Packages the GSC module.
scripts/monitoring/deploy_mcp_cloud_run.sh Enables the API and configures deployment settings.
docs/seo/SEO_AUDIT_2026-09-13.md Documents SEO findings, targets, and prioritized actions.
docs/MCP_GCP_MONITORING.md Documents GSC tools and setup.
docs/DOCUMENTATION_INDEX.md Adds the documentation index entry.
.claude/skills/seo-weekly-check/SKILL.md Defines the weekly GSC reporting workflow.
Review details

Suppressed comments (6)

scripts/monitoring/gsc_tools.py:338

  • contents[].submitted is the number of URLs submitted in the sitemap, not the number Google last read; lastDownloaded is the fetch timestamp. The current comparison therefore reports this as “URLs read” and concludes that Google has not re-read the sitemap from a count mismatch, which is not what this API field proves. Report the submitted count accurately and use the download/lastmod timestamps (or explicitly label this as a count-only heuristic), then update the duplicated output, tests, and docs.
        flags.append(f"⚠️ Clicks down {abs(comparison['clicks_pct']):.0f}% vs the previous window.")

scripts/monitoring/gsc_tools.py:410

  • Cloud Run is deployed without a key or sa_info, and ADC compute credentials commonly expose service_account_email as default, which this branch intentionally discards. A 403 then tells the operator to add "the server's credential (service account)" instead of the exact gcp-monitor-mcp@<project>.iam.gserviceaccount.com email promised by the setup instructions, so the tool output is not sufficient to complete first-run access. Propagate the runtime service-account email via configuration/metadata and use it here.
        with self._lock:
            if self._session is None:
                if self._session_factory is not None:
                    self._session = self._session_factory()
                else:

scripts/monitoring/gsc_tools.py:736

  • Any value other than the exact string "oldest" silently selects the newest URLs, while the output still labels the sample with the caller's value. A typo such as which="oldset" therefore returns the wrong coverage sample with no error. Validate the enum and use the normalized value in the selection and heading.
        return _guard(run)

scripts/monitoring/gsc_tools.py:577

  • Unlike dimension, an invalid sort_by is silently coerced to clicks. A caller asking for sort_by="position" with a typo will receive a clicks-ranked report without any indication that the requested ordering was ignored. Reject unsupported values or state the fallback in the output instead of returning misleading data.
        Optional substring filters on page URL and query text. sort_by: clicks |

scripts/monitoring/gsc_tools.py:528

  • period_windows clamps non-positive days to one day, but this helper keeps the caller's raw value for the note. For days=0 the query is one day while the report says 0d, which makes the window description inaccurate. Normalize days before both the helper call and the formatted note.
    tests can inject a fake session."""
    site_url = os.environ.get("GSC_SITE_URL", DEFAULT_SITE_URL).strip() or DEFAULT_SITE_URL
    public_base = os.environ.get("GSC_PUBLIC_BASE", DEFAULT_PUBLIC_BASE).strip() or DEFAULT_PUBLIC_BASE

scripts/monitoring/test_gsc_tools.py:207

  • This fixture hard-codes 2026-08-01 to distinguish the current and previous windows. Once the test runs after 2026-09-26, both 28-day window starts are after that cutoff, so the fake returns the current totals for both and the expected 12/8 and +50% assertions fail. Base the branch on the actual period_windows(28) result or freeze the test date.
            return FakeResponse(200, {"siteEntry": [{"siteUrl": "sc-domain:tasteslikegood.org", "permissionLevel": "siteRestrictedUser"}]})
  • Files reviewed: 10/10 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/monitoring/gsc_tools.py
Comment thread scripts/monitoring/gsc_tools.py Outdated
Comment thread scripts/monitoring/gsc_tools.py
Copilot AI review requested due to automatic review settings September 13, 2026 17:49

Copilot AI 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.

🟡 Changes recommended

Unresolved critical and moderate review findings remain in the GSC integration.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (10)

Previously missed (2) — in code that hasn't changed since the last review.

scripts/monitoring/gcp_mcp_server.py:199

  • The try only covers importing gsc_tools; _gsc_tools.register(...) runs in the else block, so any registration-time exception (for example, a FastMCP schema/dependency mismatch) aborts module import and takes the existing monitoring tools down. That contradicts the “best-effort” guarantee in the preceding comment; put the import and registration in the same try/except.
    scripts/monitoring/gsc_tools.py:692
  • GSC_PUBLIC_BASE is configurable and is used to fetch the live sitemap above, but this no-sitemap instruction hard-codes the default site's URL. With a URL-prefix property or another configured public base, the tool tells the operator to submit the wrong sitemap; build the URL from public_base here.

docs/seo/SEO_AUDIT_2026-09-13.md:190

  • URLs read inherits the incorrect interpretation of contents[].submitted; the Search Console sitemap response does not provide a read-count field there. This monitoring target should refer to the submitted URL count, while indexing should come from the coverage sample or contents[].indexed, otherwise the weekly success criterion measures the wrong field.
- Weeks 1–4: non-brand impressions > 0 and rising; sitemap "URLs read" equals the live count (98 today); `gsc_index_coverage_sample` newest 10 all PASS.

scripts/monitoring/gsc_tools.py:424

  • On the Cloud Run ADC path, sa_info is None and the compute credential can expose the service-account identity as the "default" sentinel; this branch intentionally discards it and falls back to generic text. A 403 therefore does not tell the operator which gcp-monitor-mcp@... account to add, contrary to the setup instructions. Pass the deploy-time service-account email (or resolve it from metadata) into this client instead of using the generic fallback.
                else:
                    from google.auth.transport.requests import AuthorizedSession

                    creds = build_credentials(self._sa_info)
                    self._session = AuthorizedSession(creds)

scripts/monitoring/gsc_tools.py:537

  • Credential discovery or token refresh can fail before an HTTP response exists (google.auth.default and AuthorizedSession raise their own auth exceptions), so _guard returns Search Console tool failed: ... instead of the documented actionable setup instruction. Wrap those Google auth failures in GscAccessError with the existing credential/API guidance so the first-run skill handles all access failures consistently.
# --------------------------------------------------------------------------

scripts/monitoring/gsc_tools.py:542

  • period_windows clamps the requested duration to at least one day, but this note interpolates the unnormalised argument. A call such as gsc_compare_periods(0) therefore queries a one-day window while reporting 0d, which makes the comparison output misleading; normalise days before constructing the note.
    site_url = os.environ.get("GSC_SITE_URL", DEFAULT_SITE_URL).strip() or DEFAULT_SITE_URL

scripts/monitoring/gsc_tools.py:698

  • Search Console's Sitemap.contents[].submitted is the number of URLs submitted in the sitemap; it is not a count of URLs Google read (indexed is a separate field). This output therefore mislabels the metric and makes the sitemap health readout misleading. Rename this to URLs submitted (and include indexed separately if coverage is intended).
                for r in sd

scripts/monitoring/gsc_tools.py:799

  • The weekly report repeats the same submitted/URLs read mix-up: submitted is the sitemap's submitted URL count, not a count of URLs Google read. This makes the report's indexing signal inaccurate; label it as submitted (and expose contents[].indexed separately if the report should show indexing).
            ]

scripts/monitoring/gsc_tools.py:356

  • submitted is the count from the last sitemap submission, not evidence of how many URLs Google read. The comparison can still detect that the submitted snapshot differs from the live sitemap, but this message incorrectly attributes the mismatch to Google not re-reading it. Describe it as a stale submitted snapshot (and use lastDownloaded/indexed for crawl or index status).
        if live_url_count is not None and submitted and abs(submitted - live_url_count) > 5:
            flags.append(

scripts/monitoring/gsc_tools.py:489

  • fetch_live_sitemap converts HTTP, network, and parse failures to the same empty list as a valid empty sitemap. The report then prints live sitemap: 0 and passes None to weekly_flags, so a transient public-site outage can look like an empty catalog without any warning. Preserve an explicit fetch-error state and render or flag it instead of silently treating it as zero.
    """Public fetch of sitemap.xml. None means unavailable; [] means valid but empty."""
    try:
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread scripts/monitoring/gsc_tools.py
Copilot AI review requested due to automatic review settings September 13, 2026 17:55

Copilot AI 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.

🟡 Changes recommended

A critical startup-registration issue and two moderate monitoring correctness issues remain unresolved, along with documentation nits.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

docs/seo/SEO_AUDIT_2026-09-13.md:186

  • This verification count is stale: test_gsc_tools.py contains 34 test_ methods (and the PR description reports 34), so saying 27 makes the audit's verification record inaccurate. Update the count to 34 so readers can reconcile the document with the required gate.

.github/workflows/pr-gate.yml:121

  • This new CI job now builds the monitoring image, so the requirements header's existing claim that “nothing in CI builds this image” is no longer true. Update that header to describe the current CI build/lockfile situation; otherwise future dependency changes will be guided by stale documentation.
      - name: Build monitoring MCP image
        run: docker build -f scripts/monitoring/Dockerfile scripts/monitoring

docs/seo/SEO_AUDIT_2026-09-13.md:177

  • The weekly report does not include URL Inspection or the coverage sample: it only queries analytics, sitemaps, the live sitemap, and striking-distance rows. Calling this "All of the above" overstates what one call does and may lead the routine to assume indexing coverage was checked; list the actual sections or explicitly note that inspection remains a separate tool.
| `gsc_weekly_report`         | All of the above in one call, with brand/non-brand split and ⚠️ flags               |

scripts/monitoring/gsc_tools.py:143

  • The empty-current-window case is treated as a real rank. summarize_rows([]) returns position 0.0; when the previous window has impressions, this computes a negative position_better, so gsc_compare_periods/gsc_weekly_report report a large ranking deterioration and weekly_flags adds a false position warning even though current position is undefined. Only compute this delta when cur["impressions"] is nonzero (or represent missing positions as None).
        "position_better": (prev["position"] - cur["position"]) if prev["position"] else 0.0,

scripts/monitoring/gsc_tools.py:355

  • When a sitemap record exists but contents[].submitted is zero, this guard suppresses the count comparison entirely. A live sitemap with 98 URLs and a Search Console record reporting zero submitted URLs therefore produces no mismatch flag (unless another field happens to fail), so an empty/broken submission can look healthy. Compare whenever the live count is available; a zero will naturally mismatch a non-empty live sitemap.
        if live_url_count is not None and submitted and abs(submitted - live_url_count) > 5:
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread scripts/monitoring/gcp_mcp_server.py Outdated
Comment thread docs/MCP_GCP_MONITORING.md Outdated
Comment thread docs/seo/SEO_AUDIT_2026-09-13.md Outdated
Copilot AI review requested due to automatic review settings September 13, 2026 18:00

Copilot AI 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.

🔵 Needs a closer look

Five unresolved moderate findings remain in the SEO audit and Search Console tooling.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/monitoring/gsc_tools.py:118

  • period_windows only enforces a minimum, but every public report tool passes its caller-supplied days value through this helper. An arbitrarily large MCP argument can request a range beyond Search Console's retained history or overflow datetime.timedelta, producing a generic tool failure instead of a validated request; existing monitoring tools clamp history inputs (gcp_mcp_server.py:590-600). Add an explicit upper bound supported by Search Console, or reject values outside that range before computing the dates.

docs/seo/SEO_AUDIT_2026-09-13.md:110

  • The report claims “Zero referring domains,” but its own measurement table says the Semrush backlink tools returned no data, and this paragraph only establishes that the two GitHub properties have no owned link; Docker Hub, YouTube, Instagram, and LinkedIn are explicitly unknown. That headline is unsupported by the stated evidence and can mislead prioritization; describe the external referring-domain count as unmeasured while retaining the owned-property findings.
**B1 — Zero referring domains; the owned properties do not link.** _Impact: High._ Both GitHub repos have `homepage: null`, no topics, and a README with no link to the site (0 matches). Docker Hub, YouTube, Instagram and LinkedIn exist under the brand; link status unknown. _Fix:_ one hour of bio edits; then launch posts (Show HN, Product Hunt, r/vegan) once KAN-272 ships; then outreach to the listicles that already rank for the target query. _Priority 2 (cheap)._ [KAN-277](https://tasteslikegood.atlassian.net/browse/KAN-277).

scripts/monitoring/gsc_tools.py:1314

  • The documented python scripts/monitoring/gsc_tools.py path does not use the same credential precedence as gcp_mcp_server.py: when GOOGLE_APPLICATION_CREDENTIALS points to an existing key and GOOGLE_APPLICATION_CREDENTIALS_B64 is also set, this always decodes and passes the base64 key, while the server prefers the file (gcp_mcp_server.py:124-128). The ad-hoc command can therefore query a different Search Console principal and report misleading access guidance. Only decode the base64 key when no usable file exists, matching the server.
        for _raw in _env.read_text().splitlines():
            _line = _raw.strip()
            if _line and not _line.startswith("#") and "=" in _line:
                _k, _, _v = _line.partition("=")

scripts/monitoring/gsc_tools.py:470

  • complete=False is also returned when query_all stops after a transport error or total time-budget exhaustion, not only when the page cap is reached. This helper therefore says "Sample truncated" and implies a deliberate click-ranked cap even for failed requests; use a reason-aware status or neutral wording so partial outages are not misdiagnosed.
    elif live_url_count is not None and live_sitemap_url and sitemaps:
        flags.append(f"⚠️ The configured live sitemap {live_sitemap_url} is not submitted for this property.")
    if sitemaps_available and not sitemaps:
        flags.append("⚠️ No sitemap is submitted for this property (KAN-115 submitted one on 2026-07-19 — re-check).")
    if not striking:

scripts/monitoring/gsc_tools.py:462

  • striking_complete is false both for a row-cap truncation and for the partial transport/deadline failures recorded by query_all. This message specifically claims the row cap was hit, so a failed Search Analytics request is reported as a capped sample instead of an outage; make the message describe an incomplete sample unless the caller passes the actual reason.
                comparison_sitemaps[0].get("path")
                if len(comparison_sitemaps) == 1
                else "the submitted sitemap set"
            )
            flags.append(
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 13, 2026 19:26

@adamtasteslikegood adamtasteslikegood left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed the complete 12-file diff, repository context, checks, conversation comments, submitted reviews, and all inline threads at c0fbbec99a0a399ecd6199cf2ee230bd208243ca.

No blocking findings remain.

The post-review fixes now:

  • distinguish unavailable Search Analytics and sitemap responses from genuine empty/zero data, preventing false traffic-drop and no-sitemap alarms;
  • normalize bare relative URL Inspection paths and reject unsupported schemes;
  • provide accurate Search Console property-access guidance for configured-property misses and ADC identities whose email cannot be inferred;
  • keep Cloud Monitoring IAM and Search Console property-level prerequisites distinct, with setup pointing to the complete bounded dependency set;
  • make tied mover rows deterministic with an explicit query-key tie-break.

Regression coverage verifies the partial-data, identity/property, URL-normalization, sitemap-availability, and deterministic-order cases. Exact-head validation passed: PR Gate with 59 Search Console tests, monitoring image build and 11-tool registration smoke test, frontend/backend tests and build, lint/formatting, TypeScript, Docker, canonical SEO checks, CodeQL, dependency review, all Cloud Build gates, and the independent Claude review. All 15 inline review threads are resolved.

GitHub does not allow the pull request author identity to approve its own pull request, so this clean assessment is submitted as a commit-anchored review comment.

Copy link
Copy Markdown
Owner Author

Correction to the clean review at c0fbbec: a Copilot review was submitted 20 seconds earlier but was not visible in the final consistency snapshot. It identifies five additional claims requiring verification, so the clean assessment is superseded and should not be treated as final. I am addressing the validated findings now and will post a new exact-head review after the resulting commits and checks complete.

Copilot AI 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.

🔵 Needs a closer look

Three moderate findings remain in CI transport coverage and Search Console request handling.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/monitoring/gsc_tools.py:760

  • days is only clamped to a lower bound here, so an MCP caller can pass an arbitrarily large integer. That can make date arithmetic fail with an opaque OverflowError, or request windows older than Search Console's retained history and return misleading/invalid results; the existing monitoring tools clamp their history inputs. Bound this shared window to a documented maximum (accounting for the previous comparison window), or reject out-of-range values with a clear validation message.

.github/workflows/pr-gate.yml:124

  • This smoke test only imports the server and calls list_tools(). The production image starts with MCP_TRANSPORT=http, but _run_http() (including the uvicorn/Starlette imports, transport-security setup, and mcp.streamable_http_app() call) is never exercised, so a transport-specific startup failure can pass this gate and still leave the Cloud Run connector unavailable. Add an HTTP startup smoke test with a dummy token and a stubbed uvicorn.run (or start the container and probe /healthz).
      - name: Smoke-test registered monitoring MCP tools
        run: |
          docker run --rm -i --entrypoint python gcp-monitor-mcp:test - <<'PY'

scripts/monitoring/gsc_tools.py:585

  • A successful HTTP response with an empty or non-JSON body is converted to {} here. Callers then treat it as a successful empty Search Analytics response, so gsc_weekly_report can render zero/empty sections without Partial report data when an upstream or proxy returns HTML or no body. Raise a transport/protocol error instead so the existing partial-data handling marks the affected section unavailable.
            return resp.json() if resp.text else {}
        except ValueError:
            return {}
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 13, 2026 19:32

Copilot AI 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.

🔵 Needs a closer look

URL Inspection access is documented as Restricted instead of requiring Full/Owner, and the deployment integration warrants human review.

Review details

Suppressed comments (4)

docs/MCP_GCP_MONITORING.md:399

  • Restricted access is not enough for the URL Inspection API, so the documented setup still leaves gsc_inspect_url and gsc_index_coverage_sample returning permission errors. Require a Full user (Owner also works) here; the OAuth scope can remain webmasters.readonly.
   instances. Permission **Restricted** is enough (scope is
   `webmasters.readonly`).

docs/seo/SEO_AUDIT_2026-09-13.md:184

  • This audit repeats the incorrect Restricted setup. That role is insufficient for the URL Inspection API, so the prescribed setup cannot make all eight advertised tools usable; state that Full (or Owner) is required for the inspection tools.
2. In Search Console → property `tasteslikegood.org` → Settings → Users and permissions → Add user: that service-account email, permission **Restricted**. No IAM role can substitute for this. Then call `gsc_sites`; it must list `sc-domain:tasteslikegood.org`.

scripts/monitoring/deploy_mcp_cloud_run.sh:160

  • The deployment script prints Restricted as sufficient, but the URL Inspection API requires the service account to be a Full user or Owner. As written, following this output deploys a connector where the two inspection tools fail with 403 even though gsc_sites succeeds.
echo "       permission: Restricted (read-only is all the tools need)"

scripts/monitoring/gsc_tools.py:300

  • The Restricted permission advised here is not sufficient for the URL Inspection API used by gsc_inspect_url and gsc_index_coverage_sample; that API requires a Full user or Owner. Following this instruction lets the property-list and analytics calls work but makes the inspection tools return 403, and the unknown-principal branch repeats the same advice. Update this helper and the duplicated setup instructions to require Full (or Owner).
def property_access_instruction(principal: str) -> str:
    """Actionable Search Console property-grant guidance for known/unknown identities."""
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 13, 2026 19:37

Copy link
Copy Markdown
Owner Author

Follow-up on Copilot review 5191966805: I verified the permission claim against Google's current official documentation and am not applying the suggested privilege increase.

That fetch-only capability is exactly what gsc_inspect_url and gsc_index_coverage_sample use. Full/Owner would grant broader actions that this read-only connector does not need, so the documented Restricted role remains the least-privilege setup. The four suppressed findings in that review are therefore not validated defects; they did not create inline threads to resolve.

@adamtasteslikegood adamtasteslikegood left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed the complete 12-file diff, repository context, checks, conversation comments, submitted reviews, and all inline threads at 08e97bfb5a60ba45c8a45f3ffbbc242c7ea80dbf.

No blocking findings remain.

The review-driven fixes now:

  • cap shared comparison windows at 240 days and keep displayed periods consistent with the normalized request;
  • honor a usable GOOGLE_APPLICATION_CREDENTIALS file before base64 credentials, matching server startup behavior;
  • distinguish transport/protocol failures from genuine empty results and row-cap truncation, including malformed successful HTTP responses;
  • preserve unavailable Search Analytics/sitemap state rather than generating false zero-traffic or stale-sitemap conclusions;
  • correct the SEO audit's unsupported referring-domain assertion;
  • exercise the production HTTP startup path, secret MCP route, /healthz, image build, and all 11 registered tools in the PR gate.

Validation is green at this exact head: 65 Search Console unit tests, monitoring image build and HTTP startup smoke, backend/frontend tests, build, lint/format, TypeScript, Docker, SEO checks, CodeQL, dependency review, Google Cloud Build gates, and the independent Claude review. All 15 inline review threads are resolved.

I also verified Copilot review 5191966805 against Google's official documentation. Restricted users have URL Inspection “Fetch only,” and the API implemented here only reads indexed status using webmasters.readonly; requiring Full/Owner would exceed least privilege. The supporting links and rationale are recorded in the PR conversation.

Submitted as a comment review because the connected reviewer identity is also the PR author and GitHub does not permit self-approval.

Copilot AI 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.

🔵 Needs a closer look

URL Inspection permission guidance is incorrect, gsc_compare_periods lacks end-to-end coverage, and the documented test count is stale.

Review details

Suppressed comments (6)

Previously missed (5) — in code that hasn't changed since the last review.

docs/MCP_GCP_MONITORING.md:399

  • The access instructions say Restricted is enough for the entire gsc_* set, but the URL Inspection API requires the property principal to be a Full user or Owner. As written, an operator can complete this documented setup and still receive 403s from the inspection tools; distinguish the permissions for analytics/sitemaps versus URL Inspection.
    docs/seo/SEO_AUDIT_2026-09-13.md:184
  • This audit repeats Restricted as the required permission for the whole connector, but the two URL Inspection tools in this PR require Full user or Owner. The setup checklist should not claim the tools are ready after granting Restricted; update it to the higher permission or call out the inspection exception.
    scripts/monitoring/deploy_mcp_cloud_run.sh:160
  • This deployment notice tells Adam to grant Restricted, but the deployed bundle includes URL Inspection. A Restricted user can make the analytics/sitemap calls but cannot use gsc_inspect_url or gsc_index_coverage_sample, so following this exact post-deploy instruction leaves two advertised tools unusable. Print Full user (or Owner) here, or explicitly document that inspection needs the higher permission.
    scripts/monitoring/gsc_tools.py:304
  • The URL Inspection API does not accept a Restricted Search Console user; it requires a Full user or Owner. This shared guidance is returned for 403s from gsc_inspect_url and gsc_index_coverage_sample, so the current instruction sends the operator back to a permission that cannot make those tools work. Either grant Full/Owner for this bundled connector or distinguish inspection tools from analytics-only tools.
    docs/seo/SEO_AUDIT_2026-09-13.md:186
  • This verification paragraph says 59 unit tests pass, but the added test file contains 65 test_ methods and the PR verification reports 65 passing tests. The documented verification count is stale.

scripts/monitoring/gsc_tools.py:895

  • There is no test invoking gsc_compare_periods; the suite only covers the underlying movers/summary helpers. Its two-window API calls, pagination disclosure, and rendered comparison can therefore regress while the new monitoring gate remains green. Add a fake-session end-to-end case like the other registered tools.
    def gsc_compare_periods(days: int = 28, limit: int = 10) -> str:
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

adamtasteslikegood commented Sep 13, 2026

Copy link
Copy Markdown
Owner Author

Correction to the clean review at 08e97bf: Copilot review 5192000200 arrived afterward and contains two validated findings. The audit still says 59 tests although the suite has 65, and gsc_compare_periods lacks a registered-tool end-to-end test. I am fixing both and will post a new exact-head assessment after validation.

The four repeated Full/Owner permission claims are not being applied: Google's official permission table grants Restricted users URL Inspection “Fetch only,” which matches this connector's indexed-status reads, and the method accepts webmasters.readonly. See the verification comment immediately above.

Copilot AI review requested due to automatic review settings September 13, 2026 19:48

Copy link
Copy Markdown
Owner Author

Addressed the two validated findings from Copilot review 5192000200:

  • d9b7c61 adds an end-to-end gsc_compare_periods test through the registered MCP tool. It verifies the rendered comparison plus the exact current/previous API windows, aggregate-vs-query dimensions, and row limits.
  • af40757 updates the audit verification count from 59 to 66, and the PR body now matches.

The four permission claims remain rejected for the official-docs reasons documented above. They were suppressed review-body findings, so no inline threads were created to resolve. I will post the refreshed exact-head assessment after CI completes.

@adamtasteslikegood adamtasteslikegood left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed the complete 12-file diff, repository context, exact-head checks, conversation comments, submitted reviews, and all inline threads at af40757fd46be90ae8b234d0c0e8f62355f9a729.

No blocking findings remain.

The final follow-up fixes:

  • add registered-tool end-to-end coverage for gsc_compare_periods, including rendered output, exact current/previous request windows, aggregate-versus-query dimensions, and row limits;
  • correct the audit and PR-body verification count to 66 tests.

Earlier review-driven fixes remain intact, including bounded comparison windows, correct credential precedence, protocol/error-state handling, accurate row-cap wording, normalized URL Inspection paths, least-privilege Search Console guidance, and the production HTTP startup smoke test.

All exact-head required checks inspected for this revision pass, and all 15 inline review threads are resolved. The repeated recommendation to require Search Console Full/Owner access was not applied: Google's official permissions table grants Restricted users URL Inspection “Fetch only,” matching these read-only indexed-status calls, while the API accepts webmasters.readonly. See the linked official documentation and verification in the PR conversation.

Submitted as a comment review because GitHub does not permit self-approval.

Copilot AI 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.

🔵 Needs a closer look

Four moderate access-guidance issues leave the URL Inspection tools unusable when Restricted access is granted.

Review details

Suppressed comments (4)

Previously missed (4) — in code that hasn't changed since the last review.

docs/MCP_GCP_MONITORING.md:399

  • Restricted access is sufficient for the reporting APIs but not for URL Inspection. With this instruction, the inspection tools will return 403 even though gsc_sites and Search Analytics work; document Full user (Owner also works) for the full eight-tool setup, or explicitly split the roles by capability.
    docs/seo/SEO_AUDIT_2026-09-13.md:184
  • The setup checklist repeats the same incorrect Restricted permission. Following it will leave gsc_inspect_url and gsc_index_coverage_sample failing with 403, so the audit's claim that the tools will return data is false; require Full user (Owner also works) for the complete tool set.
    scripts/monitoring/deploy_mcp_cloud_run.sh:160
  • This deployment notice tells the operator to grant Restricted access and claims that is enough for every tool. Restricted users cannot call URL Inspection, so gsc_inspect_url and gsc_index_coverage_sample remain unusable even after completing this step; print Full user (Owner also works), or limit the claim to the reporting/sitemap tools.
    scripts/monitoring/gsc_tools.py:304
  • Restricted property users cannot use the URL Inspection API, so this runtime guidance is insufficient for the two registered inspection tools: gsc_inspect_url and gsc_index_coverage_sample will still return 403 after the operator follows it. Tell operators to grant Full user (Owner also works) when enabling all tools, or split the guidance by tool capability.
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

2 participants