feat(base-action): default enableAllProjectMcpServers to false, add opt-in input

[OctavianGuzu]

Summary

Aligns base-action defaults with the principle that project-level config from the checkout should be explicitly opted into by workflow authors rather than auto-trusted.

Complements #1115 (setting_sources default) — same surface, different config knob. enableAllProjectMcpServers is written to the user-level settings file, so it applies regardless of settingSources.

Change

• base-action: enableAllProjectMcpServers now defaults to false. New enable_all_project_mcp_servers input lets workflows opt back in.
• wrapper action (action.yml): same input, default true — .mcp.json is already restored from the PR base branch by restoreConfigFromBase before execution, so wrapper-action behavior is unchanged.
• Tests and docs updated.

Behavior matrix

Caller	Before	After
claude-code-base-action (standalone)	.mcp.json servers auto-enabled	not enabled unless enable_all_project_mcp_servers: true
claude-code-action (wrapper)	.mcp.json (from base ref) auto-enabled	unchanged

[claude]

Implementation looks solid and all my earlier feedback has been addressed — deferring to a human only because this is an intentional breaking change to a security-relevant default for standalone base-action consumers, which warrants a maintainer sign-off.

Extended reasoning...

Overview

This PR changes setupClaudeCodeSettings() so that enableAllProjectMcpServers is no longer hardcoded to true; it is now driven by a new enable_all_project_mcp_servers input. The base-action defaults the input to false (a behavior change), the wrapper action defaults it to true (no behavior change, since .mcp.json is already restored from the PR base ref via restoreConfigFromBase). Touches both action.yml files, both entrypoints (base-action/src/index.ts, src/entrypoints/run.ts), the settings helper, its unit tests, the MCP integration workflow, and four doc surfaces.

Security risks

The change is a security hardening: it stops the standalone base-action from auto-approving every MCP server in a PR-controlled .mcp.json. The new input is a plain boolean string compared with === "true", plumbed via the standard composite-action env: mapping — no injection surface. The wrapper keeps its prior behavior, justified by the existing restoreConfigFromBase mitigation. I see no new exposure introduced.

Level of scrutiny

Medium-high. The code change is mechanically simple (one new optional parameter, one boolean plumbed through two action manifests), but it sits on the MCP trust boundary and is an intentional breaking change for anyone calling anthropics/claude-code-base-action directly with a project .mcp.json. The split default (false for base-action, true for wrapper) is a reasonable design but is exactly the kind of policy decision a maintainer should explicitly ratify rather than have auto-approved.

Other factors

Over four review rounds I flagged stale doc notes, a missing docs/usage.md row, and two CI regressions in test-mcp-servers.yml (including the latent mcp_config plumbing gap) — all were fixed in f3f6fb1, cf66f1d, 9d2286e, and 52fafff and the threads are resolved. Unit tests now cover both the new default-false and explicit opt-in paths, plus the override-precedence over settings JSON. The current bug-hunt pass found nothing further. The only residual item is the pre-existing mcp_config plumbing gap, which the author has explicitly scoped out with a TODO and a follow-up — that's reasonable.

[claude]

🔴 After 9452710, configRestoredFromBase is only ever set inside the isEntityContext(context) && context.isPR block — so for non-PR wrapper triggers (push, schedule, workflow_dispatch, workflow_run, and issues/issue_comment-on-an-issue) resolveEnableAllProjectMcpServers("", false) returns false and in-repo .mcp.json servers are no longer auto-enabled. Before this PR the wrapper hardcoded enableAllProjectMcpServers: true, so this is a silent behavior change for the trusted (already-merged-code) checkouts where the security rationale doesn't actually apply — and it contradicts the PR description's behavior matrix which still says wrapper is "unchanged". Either default these non-PR/trusted contexts back to true, or update the PR description / call this out as a breaking change so affected workflows know to add enable_all_project_mcp_servers: "true".

Extended reasoning...

What changed

Commit 9452710 reworked the wrapper-action default in two pieces:

• action.yml:68 — the enable_all_project_mcp_servers input default is now "" (empty), not "true" as it was in the earlier commits of this PR.
• src/entrypoints/run.ts:247-271 — configRestoredFromBase is initialized to false and only set to true inside if (isEntityContext(context) && context.isPR) { … if (restoreBase) { restoreConfigFromBase(restoreBase); configRestoredFromBase = true; } }. The result is fed to resolveEnableAllProjectMcpServers("", configRestoredFromBase).

So when the workflow doesn't set the input, the wrapper now enables project MCP servers only on PR-entity contexts where the base-ref restore actually ran. Every other event resolves to false.

Why this is a regression for non-PR wrapper users

Before this PR, run.ts called setupClaudeCodeSettings(process.env.INPUT_SETTINGS) and the function hardcoded enableAllProjectMcpServers = true. That meant a wrapper user running on push, schedule, workflow_dispatch, workflow_run, or issues with an in-repo .mcp.json got their servers auto-approved. After 9452710, those same workflows write enableAllProjectMcpServers: false to ~/.claude/settings.json, so the servers silently stop loading.

These are precisely the trusted contexts: actions/checkout on a push/schedule/workflow_dispatch event checks out the repo's own merged code, not a PR head. The PR-author-controlled-.mcp.json threat model that motivates this PR doesn't apply, yet these users lose functionality.

The PR description contradicts the shipped behavior

The PR description's behavior matrix still says:

| claude-code-action (wrapper) | .mcp.json (from base ref) auto-enabled | unchanged |

That is no longer accurate after 9452710. For PR contexts it's effectively unchanged (auto-enabled via the restore gate); for all non-PR wrapper invocations it changed from enabled → disabled. A reviewer reading the description would not know they're approving a breaking change for push/schedule/issue workflows.

On the "intentional, already documented" objection

One refutation argues this is fully intentional and documented: the JSDoc in restore-config.ts:45-52 explicitly says "Contexts where restore does not run — e.g. workflow_run, push, schedule — therefore default to false"; test/restore-config.test.ts asserts it; and action.yml / docs/configuration.md / docs/usage.md describe the new gating. That's all correct — the implementation is deliberate and the user-facing docs now describe it. But two things keep this worth flagging:

1. It's still a breaking runtime change for an existing user population (wrapper on non-PR events with in-repo .mcp.json), and nothing in the PR — title, description, or a CHANGELOG entry — calls it out as breaking. The doc updates describe the new state; they don't tell upgrading users "your push/schedule workflow will stop loading .mcp.json unless you add this input."
2. The PR description is now factually wrong about wrapper behavior. The refutation concedes this ("the only artifact that is stale is the PR description's behavior matrix"). At minimum that needs to be corrected so reviewers/maintainers know what they're merging — and ideally the author should weigh in on whether disabling in trusted non-PR contexts is actually desired vs. an over-tightening, since the security rationale (.mcp.json is PR-controlled) doesn't hold there.

Step-by-step proof

1. User has anthropics/claude-code-action@v1 triggered on schedule: (or push:, workflow_dispatch:, issues:), with an .mcp.json in the repo root and no enable_all_project_mcp_servers in with:.
2. action.yml:68 defaults the input to "" → INPUT_ENABLE_ALL_PROJECT_MCP_SERVERS = "".
3. run.ts parses context. For schedule/push/workflow_dispatch/workflow_run, isEntityContext(context) is false; for issues / issue_comment on an issue, context.isPR is false. Either way the if at run.ts:248 is skipped and configRestoredFromBase stays false.
4. resolveEnableAllProjectMcpServers("", false) → false (restore-config.ts:57-59).
5. setupClaudeCodeSettings(..., false) writes "enableAllProjectMcpServers": false to ~/.claude/settings.json.
6. Claude Code starts in the checkout, sees .mcp.json, but does not auto-approve its servers → none of the mcp__* tools are available.
7. Before this PR, step 5 would have written true unconditionally and the servers would have loaded.

Suggested fix

Either:

• Behavioral: treat non-PR / non-entity contexts as trusted and default them to true — e.g. set configRestoredFromBase (or a separate checkoutIsTrusted flag) to true when !isEntityContext(context) || !context.isPR, so only PR-head checkouts where the restore failed fall back to false. That keeps the wrapper truly "unchanged" outside the PR threat model.
• Documentation-only: update the PR description's behavior matrix to reflect that the wrapper now defaults to false on non-PR events, and add a CHANGELOG / breaking-change note so users on push/schedule/issue triggers know to add enable_all_project_mcp_servers: "true".

Either is fine, but the current state — behavior changed, description says it didn't — should not merge as-is.

[claude]

🟡 The new resolveEnableAllProjectMcpServers function (and its JSDoc) was inserted between the existing JSDoc block for restoreConfigFromBase (which ends at line 44 with @param baseBranch ...) and the restoreConfigFromBase declaration itself — so the original 20-line security-rationale doc comment is now orphaned and restoreConfigFromBase loses its hover docs. Move resolveEnableAllProjectMcpServers either above the existing JSDoc (before line 23) or below restoreConfigFromBase.

Extended reasoning...

What happened

Before this PR, lines 23-44 of src/github/operations/restore-config.ts were a single JSDoc block describing the security rationale for restoreConfigFromBase (it explains why .claude/ and .mcp.json are restored from the base branch, the RCE surface being closed, the known UX limitation, and ends with @param baseBranch - PR base branch name. Must be pre-validated...). That block was immediately followed by export function restoreConfigFromBase(baseBranch: string): void, so TSDoc/IDE tooling correctly attached the comment to that function.

This PR adds resolveEnableAllProjectMcpServers with its own /** ... */ block at lines 45-60, inserted directly after the closing */ on line 44 and before the restoreConfigFromBase declaration (which is now pushed to line 62). The file now has two back-to-back /** ... */ blocks with the new function wedged between the old doc comment and its intended target.

Why the existing comment is now orphaned

Per JSDoc/TSDoc association rules, a doc comment attaches to the declaration that immediately follows it. After this change:

• The block at lines 23-44 is followed by another comment block (line 45), not a declaration → it attaches to nothing.
• resolveEnableAllProjectMcpServers (line 53) is correctly documented by the new block at lines 45-52.
• restoreConfigFromBase (line 62) has no preceding doc comment at all — its hover/IntelliSense docs are gone.

The orphaned block also still says @param baseBranch, which makes no sense detached from any function, and the important security context ("hooks, NODE_OPTIONS, LD_PRELOAD, apiKeyHelper... are attacker-controlled") is no longer surfaced when a developer hovers restoreConfigFromBase in their IDE.

Step-by-step proof

1. Open src/github/operations/restore-config.ts at the PR head.
2. Line 44: */ closes the original JSDoc (last content line is @param baseBranch - PR base branch name...).
3. Line 45: /** immediately opens a new JSDoc block — there is no declaration between the two comment blocks.
4. Lines 53-60: export function resolveEnableAllProjectMcpServers(...) — this is the first declaration after both blocks, so only the second block (lines 45-52) attaches to it.
5. Line 62: export function restoreConfigFromBase(baseBranch: string): void — preceded by a blank line and the body of resolveEnableAllProjectMcpServers, so it has no JSDoc.
6. Hover restoreConfigFromBase in VS Code / run TypeDoc → no documentation; the @param baseBranch tag is dropped.

Impact

No runtime impact — this is purely a code-organization / doc-tooling regression. But it detaches a fairly substantial security-rationale comment from the function it describes, which is exactly the kind of context future maintainers will want surfaced in IDE hover. Hence nit.

Fix

Move resolveEnableAllProjectMcpServers (lines 45-60, including its JSDoc) to either:

• above line 23 (before the restoreConfigFromBase JSDoc), or
• below the closing brace of restoreConfigFromBase.

Either restores the original JSDoc → declaration adjacency.