fix(mcp): remove writeEnabled from Start() to prevent readonly state desync#3758
fix(mcp): remove writeEnabled from Start() to prevent readonly state desync#3758Elvand-Lie wants to merge 1 commit into
Conversation
knative-prow
Bot
added
size/M
|
Hi @Elvand-Lie. 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 Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3758 +/- ##
==========================================
+ Coverage 56.83% 56.86% +0.03%
==========================================
Files 185 185
Lines 21739 21738 -1
==========================================
+ Hits 12355 12362 +7
+ Misses 8132 8124 -8
Partials 1252 1252
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Elvand-Lie
force-pushed
the
fix/mcp-remove-start-write-param
branch
3 times, most recently
from
cfb26fb to
9e70d90
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Elvand-Lie, lkingland The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
Bot
added
the
approved
knative-prow-robot
added
the
needs-rebase
…desync The Server.Start() method unconditionally overrides the readonly state that was already set by WithReadonly() during construction. This creates a dual source of truth: instructions are computed from the readonly flag at construction time (in New()), but the actual enforcement value is overridden at Start() time. If a caller passes inconsistent values, the MCP instructions will tell the agent it is in read-only mode while the server actually permits deploy and delete operations, or vice versa. Fix by removing the writeEnabled parameter from Start() entirely. The readonly state is now set exclusively via WithReadonly() at construction time, which is also when instructions are computed. This ensures a single source of truth. Signed-off-by: Elvand Lie <elvandlie@gmail.com>
Elvand-Lie
force-pushed
the
fix/mcp-remove-start-write-param
branch
from
9e70d90 to
03ab530
Compare
|
New changes are detected. LGTM label has been removed. |
knative-prow-robot
removed
the
needs-rebase
3 participants
Summary
Fixes #3755
The
Server.Start()method unconditionally overrides thereadonlystate that was already set byWithReadonly()during construction. This creates a dual source of truth: instructions are computed from the readonly flag at construction time (inNew()), but the actual enforcement value is overridden atStart()time.If a caller passes inconsistent values, the MCP instructions will tell the agent it is in read-only mode while the server actually permits deploy and delete operations, or vice versa.
Changes
pkg/mcp/mcp.go: RemovewriteEnabledparameter fromStart(). Readonly is now set exclusively viaWithReadonly()at construction time.pkg/functions/client.go: UpdateMCPServerinterface andStartMCPServermethod to remove thewriteEnabledparameter.cmd/mcp.go: PassWithReadonly(!writeEnabled)at construction and callStartMCPServerwithout arguments.pkg/mock/mcp_server.go: Update mock to match new interface.cmd/mcp_test.go,pkg/mcp/mcp_test.go, andpkg/functions/client_test.go.Testing