fix(scheduler): enforce user hold directives by blocking mutating tools at scheduler layer - #29394

Standalone directives like "wait", "hold", or "stop" are the most direct way a user might instruct the agent to pause. However, the current regex pattern for EXPLICIT_WAIT requires trailing context words (e.g., "wait for me", "hold on"), meaning a single-word message like "wait" or "hold" will be completely ignored.

Adding a pattern specifically for standalone wait/hold/pause/stop/halt words ensures these direct user commands are programmatically enforced.

  {
    pattern:
      /\b(?:wait|hold|pause|stop|halt)\b(?:\s+(?:for|until)\s+(?:my|me|user|i\s|further)|\s+(?:before|on|here|there|now|first)|\s*[,.]?\s*(?:don'?t|do\s+not|let me))/i,
    type: HoldDirectiveType.EXPLICIT_WAIT,
    confidence: 0.95,
  },
  {
    pattern: /^(?:wait|hold|pause|stop|halt)[.,!?]*$/i,
    type: HoldDirectiveType.EXPLICIT_WAIT,
    confidence: 0.95,
  },

The negation pattern \bjust\s+(?:do|apply|fix|implement|make|go ahead|proceed)\b is overly broad because of the do keyword. If a user says "just do the analysis, don't apply yet", the phrase "just do" will trigger the negation pattern and completely disable the hold directive detection. This allows the agent to bypass the hold and aggressively mutate the codebase.

Refining the pattern to require "it" or other explicit action targets for "do" (e.g., just do it) prevents these false negatives.

The current implementation of isMutatingTool relies on checking a hardcoded set of mutating tools (MUTATING_TOOL_NAMES). This approach is fragile and insecure by default:

1. Any custom or third-party MCP tools that perform mutations (e.g., mcp_filesystem_write_file) will bypass the hold directive guard entirely.
2. Sibling tools like tracker_create_task or tracker_update_task (which modify files in the workspace) are not blocked.

To enforce a "secure by default" methodology, we should treat any tool not explicitly listed in the read-only allowlist (READ_ONLY_TOOL_NAMES) as mutating.

Since errorMessage is defined as optional (string | undefined) in the HoldDirectiveCheckResult interface, callers must handle the undefined case. Additionally, when using an optional string with a fallback value, we should trim the optional string and use the fallback if the result is empty to avoid uninformative messages from whitespace-only strings.

For consistency with nonInteractiveCliAgentSession.ts and to prevent any potential stale state if the scheduler instance is ever reused or kept alive across multiple turns/sessions in other contexts, we should explicitly clear the active hold directive when none is detected.

Critical Regex False Positive Issue

The current regex pattern for NO_CHANGES_YET makes the temporal/conditional suffix (yet, now, until, before, first) completely optional due to the trailing ?.

This causes the regex to match any negative command containing these verbs (e.g., "do not write duplicate tests", "never run commands as root", "do not change the port"). Because the scheduler programmatically blocks all mutating tools when a hold directive is active, these false positives will completely break the agent's ability to write files or run commands, failing the user's request.

Solution

Make the temporal/conditional suffix mandatory and allow optional intermediate words using [^.]*? (to keep it within the same sentence). This ensures we only match actual hold directives while ignoring general negative constraints.

The regex pattern uses [^.]*? to find a temporal suffix within the same sentence. However, [^.] matches newlines, meaning it can cross line breaks in multi-line chat messages. In chat interfaces, users often use newlines instead of periods to separate thoughts (e.g., "Do not write duplicate tests\nNow, let's implement the feature"). This will cause a false positive match on "Now" on the second line, incorrectly triggering a hold directive and blocking all mutating tools for the entire turn.

To prevent crossing newlines or other sentence boundaries, exclude newlines and other common sentence delimiters from the character class.

The pattern matches but don't change or but don't make without requiring any temporal suffix (like "yet" or "now"). This causes it to trigger false positives on common negative constraints in user prompts, such as "Fix the bug in auth.ts, but don't change the existing tests" or "Implement the feature, but don't make duplicate files". When triggered, this incorrectly activates a hold directive and programmatically blocks all mutating tools, completely preventing the agent from executing the requested task.

Since the main NO_CHANGES_YET pattern already handles "don't change ... yet" with proper temporal suffixes, we should remove the suffix-less don'?t sub-pattern from this "but" pattern to avoid these false positives.

Using the greedy wildcard .* allows matching across clause or sentence boundaries on the same line. This can lead to false positives on compound sentences where the user actually wants the agent to proceed with an action (e.g., "Show me the report. Before that, let's write the file."). In this case, the agent will match "Show me the report... before" and block the write_file tool, which is the exact action the user requested.

To prevent matching across clause or sentence boundaries, restrict the wildcard to not cross punctuation marks like periods, commas, semicolons, or question marks.

Using the greedy wildcard .* allows matching across clause or sentence boundaries on the same line. This can lead to false positives on compound sentences (e.g., "I will do it later, for now just implement the basic structure."). Here, "I will do... later" matches, triggering a hold directive and blocking all mutating tools, which prevents the agent from implementing the requested structure.

To prevent matching across clause or sentence boundaries, restrict the wildcard to not cross punctuation marks like periods, commas, semicolons, or question marks.

This pattern matches phrases like "before making any changes". If a user instructs the agent to "run the tests before making any changes", this pattern will trigger a hold directive. Because run_shell_command is classified as a mutating tool, the scheduler will block the agent from running the tests, completely defeating the user's request.

Consider refining the tool classification or the hold directive guard to allow non-destructive shell commands (like test execution or linting) to run even when a hold directive is active, or document this as a known limitation.

The pattern matches later and after in i will ..., which causes false positives when the user says they will do something "later" or "after" (e.g., "I will review the changes later, for now implement the fix"). This incorrectly blocks the agent from acting. We should restrict this pattern to first|myself to only match immediate hold directives.

Converting newlines (\n) to periods (. ) during normalization breaks multi-line messages where a newline falls inside a directive phrase (e.g., wait\nfor me or explain\nfirst), and causes false positives for sentence-start patterns like (?:^|[.!?]\s+) (e.g., run tests\nbefore making changes). Since the regex patterns already use [^.\n] to prevent crossing sentence and newline boundaries, and \s+ naturally matches newlines for intra-sentence wrapping, we should preserve newlines and only normalize horizontal whitespace.

Using [^.\n] only excludes literal periods and newlines, meaning other sentence/clause boundaries like ! and ? are not excluded. This allows the regex to match across sentence boundaries, leading to false positives where a hold directive in one sentence is incorrectly combined with an action directive in the next sentence (e.g., 'do not apply! Now, let's write the file.'). Excluding ! and ? by using [^.!?\n] instead of [^.\n] prevents these false positives.

Similar to the previous pattern, using [^.!?\n] instead of [^.\n] ensures that sentence boundaries like ! and ? are respected, preventing false positives on compound sentences.

Using [^.!?\n] instead of [^.\n] prevents matching across sentence boundaries marked by ! or ?.