fix: make MCP readonly state immutable after construction

[SeeyaVhora]

Summary

This PR fixes an MCP readonly-state consistency issue where the server readonly mode could be changed during Start(), even after MCP instructions and tool capabilities had already been generated during New().

Previously:

• readonly state was configured during construction using WithReadonly(...)
• MCP instructions/tool exposure were derived from that initial readonly state
• Start(ctx, writeEnabled bool) could later silently override the readonly value

This created a lifecycle inconsistency where:

• MCP instructions could describe readonly behavior,
• while the runtime server state could become writeable.

Fixes #3785

---

Root cause

The MCP server generated instructions/tool exposure during construction:

s.instructions = buildInstructions(s.readonly)

However, Start() later mutated the readonly state again:

s.readonly.Store(!writeEnabled)

This allowed the effective runtime enforcement state and the already-generated MCP instructions to drift apart.

---

What changed

Single source of truth for readonly state

Readonly state is now immutable after construction.

Changes:

• Start(ctx, writeEnabled bool) → Start(ctx)
• removed runtime readonly mutation
• readonly configuration is now handled exclusively during server creation via WithReadonly(...)

This removes the possibility of instruction/runtime desynchronization entirely.

Deterministic lifecycle behavior

Added a lightweight idempotency guard using atomic.Bool so repeated Start() calls become safe no-op operations.

MCP capability consistency

Mutating MCP tools (deploy, delete) are now:

• registered only in writeable mode
• omitted entirely in readonly mode

This keeps exposed MCP capabilities aligned with effective runtime behavior.

Behavioral invariant tests

Added/refined behavioral MCP tests that:

• validate readonly/writeable tool exposure through real MCP protocol interactions
• verify deterministic lifecycle behavior
• verify readonly behavior remains stable across the server lifecycle

The tests intentionally avoid:

• reflection
• unsafe access
• private field inspection
• instruction string parsing

and instead validate behavior through MCP capability exposure.

---

Behavior changes

Behavior	Before	After
Readonly source	WithReadonly(...) + Start(..., writeEnabled)	WithReadonly(...) only
Runtime readonly mutation	Allowed	Removed
MCP tool exposure in readonly mode	Tools registered then rejected	Tools omitted entirely
Repeated Start() calls	Multiple initialization attempts possible	Safe no-op
Instruction/runtime drift	Possible	Structurally impossible

---

Why this approach

The goal was to fix the root cause by simplifying the lifecycle model instead of adding runtime validation or synchronization logic.

This change:

• removes dual readonly-state authority
• makes invalid states structurally impossible
• reduces lifecycle complexity
• preserves deterministic behavior
• keeps runtime overhead minimal
• keeps MCP capability exposure aligned with enforcement behavior

---

Verification

Verified with:

go test -v ./pkg/mcp/...
go test -v ./cmd/...
go test -v ./pkg/functions/...
make check
make test

All relevant MCP, CLI, and client tests pass successfully.

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SeeyaVhora
Once this PR has been reviewed and has the lgtm label, please assign gauron99 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:

• OWNERS

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

[knative-prow]

Welcome @SeeyaVhora! It looks like this is your first PR to knative/func 🎉

[knative-prow]

Hi @SeeyaVhora. 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.

Regular contributors should join the org to skip this step.

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.

[SeeyaVhora]

@gauron99 @lkingland Please review this PR and let me know to make any changes.

[Ankitsinghsisodya]

Your PR is duplicate Ig #3758

[lkingland]

Thanks for the careful fix and tests, @SeeyaVhora — this is well-shaped work. Closing as a duplicate: PR #3758 (by @Elvand-Lie) targets the same MCP readonly-state desync bug (tracked separately as #3755, which #3785 also duplicates) and has been approved/lgtm'd. I want to avoid splitting maintainer review effort. The test coverage you added here is actually more thorough than #3758's — if you'd like to land that as a follow-up PR after #3758 merges, that'd be a welcome contribution.

[SeeyaVhora]

Got it, thanks for the clarification and for linking the related work.
I could still be useful as smaller follow-up contributions around the same area.

Appreciate the review and feedback.