[addyCooks]

Description

Development mode only decided which tools the model was shown. A model that named a hidden tool anyway (write_file or execute_bash in plan mode) went straight to approval, where an alwaysAllow entry skipped the prompt and an ACP client's Allow ran it. With alwaysAllow set, plan mode wrote files and ran commands with no prompt, in both nanocoder run and ACP.

The interactive loop and the plain runtime now refuse a registered tool that was not in the turn's offered list, through the shared tool-call partition, and send the model a "not available in the current mode" result. ACP's per-call guard, which only caught a tool that was offered and then hidden by a mode switch, now refuses any registered tool the session's current mode does not offer. Unknown names still take the existing unknown-tool path, and subagents already re-check availability. This also covers MCP tools that plan mode hides because they are not read-only.

Tests cover the partition, and ACP plan-mode turns with a client that allows the call and with alwaysAllow. They fail without the fix. The ACP mock tool managers now offer the tools their tests run, as the real manager does.

Found during a smoke test of main at 90ffb693. Related to #989, which fixed the same class of problem for MCP only.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully)
• Tests cover both success and error scenarios

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md)

[github-actions]

nc-review: comments — 1 important

@addyCooks — a few things worth a look, none blocking.

The PR closes a real gap: a registered tool call that the active mode never offered could still run, because availability was only enforced by the offered tool list, not at the execution boundary. Using the shared partitionUnknownToolCalls/filterValidToolCalls path is the right shape - it keeps the assistant message and its tool results paired 1:1, so the model gets the refusal and can self-correct, and routing availability through ToolManager.getAvailableToolNames keeps one authority for mode, tune-profile and disabled-tool filtering. The one thing worth fixing before merge is the interactive path, which validates against a turn-start snapshot of the offered list rather than the live mode; the ACP and subagent paths this PR also touches both read the mode live.

🟠 important · source/hooks/chat-handler/conversation/conversation-loop.tsx:379

The offered list this PR validates against is computed once, at the top of processAssistantResponse, from params.developmentMode:

const availableNames = toolManager?.getAvailableToolNames(
    tune,
    developmentMode,
    undefined,
    currentModel,
) ?? [];

developmentMode here is the developmentMode prop captured when the outermost turn started. Every recursion site re-enters with {...params} (lines 527, 857, 1244, 1318, 1406, 1436), so the snapshot is never refreshed for the rest of the turn. The live mode is available separately as developmentModeRef, and the same function already prefers it for the approval decision (mode: developmentModeRef?.current ?? developmentMode, line 959 and line 1073) - but the availability check does not use it.

That gap is reachable, because switching modes mid-turn is allowed: source/components/user-input.tsx:1015 handles Shift+Tab as "always available", including while a turn is generating. So this sequence is possible:

1. The turn starts in normal (or yolo), and availableNames includes write_file / execute_bash / file_op.
2. The model emits a call to one of them.
3. The user presses Shift+Tab and switches to plan, which excludes every mutating tool (MODE_EXCLUDED_TOOLS.plan, source/tools/tool-manager.ts:45-51).
4. The PR's filter checks the call against the turn-start list, which still contains the tool, so the call is treated as valid and proceeds.
5. resolveToolApproval is then asked with mode: 'plan', but alwaysAllow short-circuits first (source/tools/approval-policy.ts:50-53), before any mode is considered. With write_file in nanocoder.alwaysAllow - a documented, ordinary setting (docs/configuration/index.md:392-402) - no prompt appears and the mutating call executes under plan mode.

This is the same posture the documentation states for MCP tools: "alwaysAllow cannot override plan mode. Plan mode exists to inspect a model's intentions without side effects" (docs/configuration/mcp-configuration.md:241-245). The enforcement this PR adds in the interactive path does not hold that line for built-in tools.

The two other paths the PR touches do not have this problem, because they check availability against the mode as it is now: ACP compares against session.developmentMode read live (source/acp/acp-conversation.ts:583-591), and the subagent executor checks its allow-list against bridge?.getMode() (source/subagents/subagent-executor.ts:1038-1044). The interactive path is the one place where the new guard can be stale.

Suggested fix: derive the validation list from the live mode rather than the turn-start snapshot - i.e. compute the availability check against developmentModeRef?.current ?? developmentMode at the point the tool calls are validated, not once at the top of the turn. If a single list cannot be used for both offering and validating, validate against a freshly computed one and keep the turn-start list only for the tools actually offered to the model. A regression test would cover the mid-turn switch: offer write_file in normal, flip the mode to plan before the call is validated, and assert the call is refused.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}

[addyCooks]

/re-review

[github-actions]

nc-review: nothing to raise

@addyCooks — nothing to raise from the automated review.

The PR closes a real gap: before it, availability was only enforced by the offered tool list, so a model naming a hidden tool (e.g. write_file in plan mode) reached approval, where alwaysAllow/yolo skipped the prompt and the call ran. The shared partitionToolCalls now also refuses a registered tool that the current mode does not offer, and the interactive loop, nanocoder run and ACP all check the live mode. The previous review's one important finding is resolved: the interactive path no longer validates against a turn-start snapshot - it derives the list through liveAvailableNames(), which reads developmentModeRef?.current ?? developmentMode, and developmentModeRef.current is refreshed on every render of useAppState. The regression tests cover the partition, a mid-turn switch to plan in the interactive loop (before the response, between two tools of a batch, and before a confirmation prompt), and ACP plan-mode turns with a client that would allow the call and with alwaysAllow; each would fail without the fix. No false refusals are introduced - getAvailableToolNames is derived from the registry, so it is a superset of what getFilteredTools offers, and scoped skill tools stay in it. The ACP path's per-call guard is a deliberate divergence from the abandon-the-turn partition and still refuses the tool, so it is a design difference rather than a hole. The changeset is present and its package name resolves against the workspace.

Addressed since the last review

• ✅ The interactive path no longer validates tool calls against a stale turn-start snapshot of the offered list: liveAvailableNames() in source/hooks/chat-handler/conversation/conversation-loop.tsx reads developmentModeRef?.current ?? developmentMode at the point of use, and developmentModeRef.current is updated on every render in source/hooks/useAppState.tsx, so a mid-turn Shift+Tab to plan is seen by the guard.
• ✅ The same live check is applied at the auto-batch beforeExecute gate and at the confirmation-prompt loop, so alwaysAllow cannot run a tool the new mode excludes, and a tool hidden between two calls of a batch is refused before it runs.
• ✅ Regression tests cover the mid-turn switch: source/hooks/chat-handler/conversation/conversation-loop.spec.ts now asserts that a switch to plan while the model answers refuses a call alwaysAllow would have skipped, that a switch between two tools of a batch stops the second, and that a switch before a confirmation prompt means the tool is never asked about.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}