Harden script execution, value semantics, and LSP diagnostics

CHANGELOG.md

@@ -8,10 +8,20 @@
 - Added `$sessionRequestCharge` and `$sessionChargedOperationCount` as read-only shell variables. Set `$sessionRequestChargeWarningThreshold` to a positive RU threshold to print one warning when the current connection reaches it; `info` reports it as `session.requestChargeWarningThreshold`.
 - Destructive MCP confirmations now identify their target. The elicitation prompt adds the connected account endpoint and the current database/container location, and notes that explicit `--db`/`--con` arguments override that location. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
 - Import and export no longer hold entire files in memory. CSV imports are parsed incrementally, and CSV exports spool documents to a private temporary file to determine the complete column set, so transfers no longer scale with document count. Allow temporary disk space for the CSV export spool in addition to the destination file. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
+- Script diagnostics now report where a failure happened. Human-readable output shows the innermost source location first followed by the recorded function and script call sites, JSON errors carry the originating file, line, and column, and diagnostic logs retain both through the existing secret-redaction pipeline. Functions keep their defining file's location even when invoked from another file. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- The language server now applies the same validation as script execution: control-flow placement, duplicate function parameters, document-local function names, and commands and built-in options nested inside blocks, branches, loops, pipelines, and command expressions. Variable and function symbols are case-sensitive, so `$value` and `$Value` stay distinct. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Host-requested cancellation now propagates through script files, blocks, loops, and function calls without being turned into a positional runtime error. The shell reports a neutral result, records the cancellation in the diagnostic log, and restores call scopes and source context. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Documented the shell language in [programming](docs/programming.md): operator precedence and associativity, compound assignment, numeric promotion, a statement grammar, validation rules, and resource limits. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
 
 ### Breaking changes
 
 - Malformed CSV files are now rejected instead of being silently misread. An unterminated or misplaced quote previously caused the remainder of the file to be absorbed into a single field, so the import reported success while writing corrupted items. Such files now abort with `Invalid CSV record at line <n>`. Imports that previously appeared to succeed may now fail and require the source file to be corrected. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
+- Command text and script files are now fully parsed and validated before any of their statements run, so a syntax or semantic error prevents the entire input from executing rather than failing part-way through. Invalid control flow is rejected: `return` requires an enclosing function or script file, `break` and `continue` require an enclosing loop in the same function or script, and duplicate function parameter names are refused. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Calling a function with too few or too many arguments is now a usage error that exits with code `2`, including calls inside expressions. The function body does not run. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Integer arithmetic now reports overflow as an error instead of wrapping. Integer literal magnitudes must be between `0` and `2147483647`; because the minus sign is a separate unary operator, the minimum integer must be written as `-2147483647 - 1`. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- JSON construction now preserves decimal types, so `$object = {"value":3.0}` stores JSON `3.0` and `$object.value / 2` produces `1.5`. It previously stored `3` and performed integer division, producing `1`. Scripts that relied on the old truncation must be reviewed. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- A failed command expression now propagates its error instead of silently producing an empty result. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Scripts are subject to fixed resource limits: a shared parser nesting budget of 128 entries, a maximum expression tree depth of 128 nodes, and at most 64 active function and script-file calls. Exceeding a limit fails with a diagnostic instead of continuing recursive parsing or execution. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
 
 ### Fixes
 
@@ -21,6 +31,11 @@
 - Shell and MCP command execution is serialized, including nested shell calls, so concurrent requests can no longer interleave and corrupt the shared connection and navigation state. Waiting for a destructive confirmation does not hold the execution lock. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
 - A destructive MCP command is refused when the connection or navigation context changes while its confirmation is pending, including navigating away and back. It previously ran against the changed context. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
 - Echoing an MCP command line no longer fails the command it announces on hosts without an ANSI terminal, which previously reported `Terminal does not support ANSI` instead of running it. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
+- `export` now rejects a directory as its destination and rejects an existing file before running its query instead of after. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- `return` no longer leaves the previous statement's custom renderer and explicit output format active, which could display an earlier command's output in place of the returned value. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Attaching a source location to a runtime failure no longer changes its exit-code category, so authentication, throttling, connectivity, and arithmetic failures are no longer reported as usage errors. Parser errors raised from a script file keep that file's name and source text, including when reached through a command expression. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- Loops and functions preserve JSON `null` values, and numeric conditions use the same zero/nonzero rule for shell values and JSON properties, including fractional numbers. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
+- The language server records a `do` loop's body before its condition and records a `for` binder as the loop variable's definition, so hover and go-to-definition no longer resolve to the wrong occurrence. ([#208](https://github.com/Azure/CosmosDBShell/pull/208))
 - MCP command lines now list positional arguments in the order the command binds them. A destructive confirmation and the recorded history entry previously followed the client's argument order, so `rmdb` could display its `force` flag in place of the database name. A call that supplies a positional argument while omitting an earlier one is now rejected, because the shell cannot express that call and the recorded command would bind differently on replay. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
 - MCP invocations are now saved to the history file as they run and are bounded by the history size limit. They were previously saved only when a later interactive command was entered. On Linux and macOS, the history file is now restricted to its owner, including an existing file that was previously readable by other users. ([#207](https://github.com/Azure/CosmosDBShell/pull/207))
 

CONTRIBUTING.md

@@ -89,6 +89,18 @@ docs/                                        # User-facing documentation
 - **Tests** live in `CosmosDBShell.Tests/`. Add or update tests when changing behavior.
 - Match the existing C# style. Prefer clear names over abbreviations.
 
+### Script Language Tests
+
+When changing value semantics, extend `ValueOriginCases` in [StatementExecutionTests.cs](CosmosDBShell.Tests/Parser/StatementExecutionTests.cs). The matrix evaluates each operand as a literal, a JSON property, a loop variable, and a function argument/return, checking both result type and value across all combinations. Each combination runs with both raw JSON fixtures and shell-constructed JSON so serialization changes cannot silently alter the input's numeric type. Include repeated object/array reconstruction and integral decimal values when changing JSON conversion.
+
+Exercise syntax and control flow through `ShellInterpreter.RunCommandAsync` or script-file execution, which includes production parse/semantic validation. Check that invalid statements prevent execution and that returns, failures, and cancellation restore call scopes. For editor changes, check symbol identity, definition/reference locations, and hover ranges as well as diagnostics.
+
+Run the offline regression suite without a database:
+
+```bash
+dotnet test CosmosDBShell.Tests/CosmosDBShell.Tests.csproj --filter "Category!=Emulator"
+```
+
 ### Updating Localized Text
 
 Edit `CosmosDBShell/lang/en.ftl`, then build normally:

CosmosDBShell.Tests/CommandTests/CosmosCommandTests.cs

@@ -72,6 +72,18 @@ await Assert.ThrowsAsync<OperationCanceledException>(() => shell.ExecuteCosmosCo
         Assert.Equal(1, shell.SessionChargedOperationCount);
     }
 
+    [Fact]
+    public async Task ExecuteCosmosCommandAsync_RunsInsideSerializedOperation()
+    {
+        using var shell = ShellInterpreter.CreateInstance();
+
+        var state = await shell.RunSerializedAsync(
+            () => shell.ExecuteCosmosCommandAsync(new TestCosmosCommand(2.5), new CommandState(), string.Empty, CancellationToken.None),
+            CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
+
+        Assert.Equal(2.5, state.RequestCharge);
+    }
+
     [Fact]
     public void CreatePartitionKey_WithHierarchicalIntegerComponents_PreservesIntegerTypes()
     {

CosmosDBShell.Tests/CommandTests/ExportCommandTests.cs

@@ -10,6 +10,7 @@ namespace CosmosShell.Tests.CommandTests;
 using System.Threading;
 using System.Threading.Tasks;
 using Azure.Data.Cosmos.Shell.Commands;
+using Azure.Data.Cosmos.Shell.Util;
 using Microsoft.Azure.Cosmos;
 using NSubstitute;
 
@@ -259,7 +260,14 @@ Task<int> ExportAsync() => ExportCommand.WriteFileAsync(
             }
             else
             {
-                await Assert.ThrowsAsync<IOException>(ExportAsync);
+                var exception = await Assert.ThrowsAsync<IOException>(ExportAsync);
+                Assert.Equal(
+                    MessageService.GetArgsString("command-export-error-file_exists", "file", Path.GetFullPath(path)),
+                    exception.Message);
+                var items = Substitute.For<IAsyncEnumerable<JsonElement>>();
+                await Assert.ThrowsAsync<IOException>(() => ExportCommand.WriteFileAsync(
+                    items, ExportFormat.JsonLines, path, false, TestContext.Current.CancellationToken));
+                items.DidNotReceive().GetAsyncEnumerator(Arg.Any<CancellationToken>());
                 Assert.Equal("previous export", await File.ReadAllTextAsync(path, TestContext.Current.CancellationToken));
             }
 
@@ -271,6 +279,32 @@ Task<int> ExportAsync() => ExportCommand.WriteFileAsync(
         }
     }
 
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public async Task WriteFileAsync_ExistingDirectoryDoesNotEnumerateItems(bool overwrite)
+    {
+        var directory = Directory.CreateTempSubdirectory("cosmos-export-test-");
+        try
+        {
+            var items = Substitute.For<IAsyncEnumerable<JsonElement>>();
+
+            var exception = await Assert.ThrowsAsync<IOException>(() => ExportCommand.WriteFileAsync(
+                items, ExportFormat.JsonLines, directory.FullName, overwrite, TestContext.Current.CancellationToken));
+
+            Assert.Equal(
+                MessageService.GetArgsString("command-export-error-destination_directory", "file", Path.GetFullPath(directory.FullName)),
+                exception.Message);
+            Assert.Contains("Specify a file path instead", exception.Message, StringComparison.Ordinal);
+            Assert.DoesNotContain("--force", exception.Message, StringComparison.Ordinal);
+            items.DidNotReceive().GetAsyncEnumerator(Arg.Any<CancellationToken>());
+        }
+        finally
+        {
+            directory.Delete();
+        }
+    }
+
     [Fact]
     public async Task WriteFileAsync_CancellationPreservesExistingFile()
     {

CosmosDBShell.Tests/Integration/ScriptArgumentTests.cs

@@ -86,14 +86,16 @@ public async Task Script_MultiLineWithControlFlow()
         var tempDir = Path.Combine(Path.GetTempPath(), "CosmosShellIntTests", Guid.NewGuid().ToString("N"));
         Directory.CreateDirectory(tempDir);
         var scriptPath = Path.Combine(tempDir, "control_flow.csh");
-        await File.WriteAllTextAsync(scriptPath, "$sum = 0\nfor $i in [1, 2, 3] {\n  $sum = ($sum + $i)\n}\nreturn $sum\n", TestContext.Current.CancellationToken);
+        await File.WriteAllTextAsync(scriptPath, "$sum = 0\nfor $i in [1, 2, 3] {\n  $sum = ($sum + $i)\n}\nreturn $sum\ndef unreachable { return 0 }\n", TestContext.Current.CancellationToken);
 
         try
         {
             var cmd = new CommandStatement(new Token(TokenType.Identifier, scriptPath, 0, scriptPath.Length));
             var state = await cmd.RunAsync(Shell, new CommandState(), CancellationToken.None);
 
             Assert.False(state.IsError);
+            Assert.Equal(6, Assert.IsType<ShellNumber>(state.Result).Value);
+            Assert.False(Shell.Functions.ContainsKey("unreachable"));
         }
         finally
         {

CosmosDBShell.Tests/Integration/ShellProcessTests.cs

@@ -24,6 +24,40 @@ public class ShellProcessTests
 {
     private static readonly Regex AnsiEscape = new("\x1b\\[[0-9;?]*[ -/]*[@-~]", RegexOptions.Compiled);
 
+    [Theory]
+    [InlineData("value", "identity", "1 argument, got 0")]
+    [InlineData("value", "identity 1 2", "1 argument, got 2")]
+    [InlineData("value", "$result = (identity)", "1 argument, got 0")]
+    [InlineData("", "identity 1", "0 arguments, got 1")]
+    [InlineData("first second", "identity", "2 arguments, got 0")]
+    public async Task WrongFunctionArgumentCount_ReturnsUsageExitCode(string parameters, string invocation, string expectedMessage)
+    {
+        var result = await RunShellAsync($"def identity [{parameters}] {{ return }}; {invocation}", cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]);
+        Assert.Equal(2, result.ExitCode);
+        Assert.Contains("expects " + expectedMessage, result.StdErr);
+    }
+
+    [Fact]
+    public async Task DeepExpression_FailsBeforeExecution_WithoutCrashingProcess()
+    {
+        var script = "echo BEFORE_DEPTH_ERROR; $value = " + string.Join(" + ", Enumerable.Repeat("1", 10001));
+        var result = await RunShellAsync(script, cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]);
+        Assert.Equal(2, result.ExitCode);
+        Assert.DoesNotContain("BEFORE_DEPTH_ERROR", result.StdOut);
+        Assert.Contains("expression tree depth", result.StdErr);
+        Assert.DoesNotContain("Stack overflow", result.StdErr);
+    }
+
+    [Fact]
+    public async Task ExpressionAtDepthLimit_EvaluatesSuccessfully()
+    {
+        var script = "$value = " + string.Join(" + ", Enumerable.Repeat("1", 128)) + "; echo $value";
+        var result = await RunShellAsync(script, cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]);
+        Assert.Equal(0, result.ExitCode);
+        Assert.Contains("128", result.StdOut);
+        Assert.Empty(result.StdErr);
+    }
+
     [Theory]
     [InlineData("doctor --no-update-check --format json", 0, "PASS")]
     [InlineData("doctor --database missing --no-update-check --format json", 1, "FAIL")]

CosmosDBShell.Tests/Lsp/CosmosShellCompletionHandlerTests.cs

@@ -98,6 +98,26 @@ public async Task VariableCompletion_SuggestsSessionVariables()
         Assert.Contains("$sessionRequestChargeWarningThreshold", labels);
     }
 
+    [Fact]
+    public async Task VariableCompletion_PreservesNamesThatDifferOnlyByCase()
+    {
+        var variables = new VariableContainer();
+        variables.Set("completionValue", new ShellText("lower"));
+        variables.Set("CompletionValue", new ShellText("upper"));
+        ShellInterpreter.Instance.VariableContainers.Push(variables);
+        try
+        {
+            var completions = await GetCompletionsAsync("echo $completion", 0, 16);
+
+            Assert.Single(completions.Items, item => item.Label == "$completionValue" && item.InsertText == "$completionValue");
+            Assert.Single(completions.Items, item => item.Label == "$CompletionValue" && item.InsertText == "$CompletionValue");
+        }
+        finally
+        {
+            ShellInterpreter.Instance.VariableContainers.Pop();
+        }
+    }
+
     [Fact]
     public async Task VariableCompletion_IgnoresWhenNotVariableContext()
     {

CosmosDBShell.Tests/Lsp/CosmosShellHoverHandlerTests.cs

@@ -133,6 +133,34 @@ public async Task Handle_VariableHover_ReturnsVariableInfo()
         Assert.NotNull(result);
         var markup = Assert.IsType<MarkupContent>(result.Contents.MarkupContent);
         Assert.Contains("myVar", markup.Value);
+        Assert.Equal(new Position(1, 5), result.Range!.Start);
+        Assert.Equal(new Position(1, 11), result.Range.End);
+    }
+
+    [Theory]
+    [InlineData("value", 2, 0)]
+    [InlineData("Value", 3, 1)]
+    public async Task Handle_VariableHover_UsesCaseSensitiveDefinition(string name, int usageLine, int definitionLine)
+    {
+        const string content = "$value = 1\n$Value = 2\necho $value\necho $Value";
+        this.workspace.OpenDocument(testUri, content, 1);
+        var document = this.workspace.GetDocument(testUri)!;
+        var symbol = document.SemanticModel!.GetSymbolAt(content.LastIndexOf("$" + name, StringComparison.Ordinal) + 1)!;
+        Assert.Equal(name, symbol.Name);
+        var definition = Assert.Single(document.SemanticModel.FindReferences(symbol), reference => reference.IsDefinition);
+        Assert.Equal(definitionLine == 0 ? 0 : content.IndexOf('\n') + 1, definition.Start);
+
+        var result = await this.handler.Handle(new HoverParams
+        {
+            TextDocument = new TextDocumentIdentifier { Uri = testUri },
+            Position = new Position(usageLine, 6),
+        }, TestContext.Current.CancellationToken);
+
+        Assert.NotNull(result);
+        var markup = Assert.IsType<MarkupContent>(result.Contents.MarkupContent);
+        Assert.Contains(name, markup.Value);
+        Assert.Equal(new Position(usageLine, 5), result.Range!.Start);
+        Assert.Equal(new Position(usageLine, 11), result.Range.End);
     }
 
     [Fact]
@@ -451,6 +479,8 @@ public async Task Handle_FunctionCall_ReturnsFunctionInfo()
         Assert.NotNull(result);
         var markup = Assert.IsType<MarkupContent>(result.Contents.MarkupContent);
         Assert.Contains("myFunc", markup.Value);
+        Assert.Equal(new Position(3, 0), result.Range!.Start);
+        Assert.Equal(new Position(3, 6), result.Range.End);
     }
 
     [Fact]