Skip to content

refactor(mcp): call pkg/functions directly for config list tools#3792

Open
xenonnn4w wants to merge 1 commit into
knative:mainfrom
xenonnn4w:mcp-config-list
Open

refactor(mcp): call pkg/functions directly for config list tools#3792
xenonnn4w wants to merge 1 commit into
knative:mainfrom
xenonnn4w:mcp-config-list

Conversation

@xenonnn4w
Copy link
Copy Markdown
Contributor

Describe

Migrates the MCP config list tools, config_envs_list, config_labels_list, and config_volumes_list, from shelling out to func config ... (and parsing stdout) to loading the function directly via pkg/functions.NewFunction.

This is the next slice of the broader pkg/mcp -> pkg/functions migration tracked in

Changes

  1. pkg/mcp/tools_config_envs.go
  2. pkg/mcp/tools_config_labels.go
  3. pkg/mcp/tools_config_volumes.go
  • Updates each list handler to call fn.NewFunction(path) and read configured envs/labels/volumes directly from the fn.Function struct.

  • ConfigEnvsListOutput, ConfigLabelsListOutput, and ConfigVolumesListOutput now carry typed structured data into CallToolResult.StructuredContent.

  • Keeps Message as a short human-readable fallback summary.

  • Adds MCP-local DTOs:

    • EnvVar
    • LabelPair
    • VolumeMount
    • VolumePVC
    • VolumeEmptyDir

    These mirror the corresponding fn types and are required because the MCP SDK output-schema generator cannot parse the invopop/jsonschema struct tags present on fn.Env, fn.Label, and fn.Volume.

  • Removes the Verbose input from all three tools since it only controlled subprocess logging and no longer applies once the calls are in-process.

Tests

  • Rewrites config list tests to create a real function project in a temporary directory and assert structured output is populated correctly.
  • Adds a shared writeTestFunction helper for constructing test functions.
  • Replaces executor-error tests with nonexistent-path cases, since errors now originate from fn.NewFunction rather than subprocess execution.

Other MCP tools (config add/remove, build, deploy, etc.) continue using the executor interface for now and will migrate incrementally as part of the phased plan in

Related Issues

Additional reviewer notes

  • No cmd/mcp.go change:
    config list tools read func.yaml directly from disk via package-level fn.NewFunction, so no *fn.Client or WithClientProvider plumbing is required.

  • refactor(mcp): call pkg/functions directly for list tool #3778 is referenced only for migration context:
    this branch is based directly on upstream/main and merges independently.

VI. Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests where applicable
  • All checks passed in make test where applicable

Migrate the config_envs_list, config_labels_list and config_volumes_list
MCP tools from shelling out to `func config ...` to reading the function
directly via pkg/functions.NewFunction.

Results are returned as typed structured data in StructuredContent. The
CLI-only `verbose` input is dropped.
@knative-prow knative-prow Bot requested review from dsimansk and jrangelramos May 20, 2026 11:57
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented May 20, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: xenonnn4w
Once this PR has been reviewed and has the lgtm label, please assign lkingland for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow Bot added size/L 🤖 PR changes 100-499 lines, ignoring generated files. needs-ok-to-test 🤖 Needs an org member to approve testing labels May 20, 2026
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented May 20, 2026

Hi @xenonnn4w. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 72.36842% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.45%. Comparing base (2d776b0) to head (98cddc4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/mcp/tools_config_volumes.go 58.33% 12 Missing and 3 partials ⚠️
pkg/mcp/tools_config_envs.go 85.00% 2 Missing and 1 partial ⚠️
pkg/mcp/tools_config_labels.go 85.00% 2 Missing and 1 partial ⚠️

❗ There is a different number of reports uploaded between BASE (2d776b0) and HEAD (98cddc4). Click for more details.

HEAD has 10 uploads less than BASE
Flag BASE (2d776b0) HEAD (98cddc4)
e2e 3 2
e2e-config-ci 1 0
e2e node 1 0
e2e typescript 1 0
e2e springboot 1 0
e2e go 1 0
e2e python 1 0
e2e quarkus 1 0
e2e rust 1 0
integration 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3792       +/-   ##
===========================================
- Coverage   56.83%   45.45%   -11.39%     
===========================================
  Files         185      200       +15     
  Lines       21739    23478     +1739     
===========================================
- Hits        12355    10671     -1684     
- Misses       8132    11859     +3727     
+ Partials     1252      948      -304     
Flag Coverage Δ
e2e 21.58% <0.00%> (-14.19%) ⬇️
e2e go ?
e2e node ?
e2e python ?
e2e quarkus ?
e2e rust ?
e2e springboot ?
e2e typescript ?
e2e-config-ci ?
integration ?
unit macos-14 42.23% <76.66%> (-2.66%) ⬇️
unit macos-latest 42.23% <76.66%> (-2.66%) ⬇️
unit ubuntu-24.04-arm 42.54% <72.36%> (-2.57%) ⬇️
unit ubuntu-latest 43.10% <76.66%> (-2.72%) ⬇️
unit windows-latest 42.28% <76.66%> (-2.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test 🤖 Needs an org member to approve testing size/L 🤖 PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant