Skip to content

Centralize shell output independently of MCP stdio - #254

Open
Mike Krüger (mkrueger) wants to merge 20 commits into
mainfrom
dev/mkrueger/centralize-shell-output
Open

Mike Krüger (mkrueger) wants to merge 20 commits into
mainfrom
dev/mkrueger/centralize-shell-output

Conversation

@mkrueger

@mkrueger Mike Krüger (mkrueger) commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Extract the independently useful CLI output improvements from #239 while deferring MCP stdio entirely.

  • Centralize text, markup, renderables, exceptions, and terminal clearing through ShellOutput and a category-based output policy.
  • Keep --quiet informational output silent while preserving command results, warnings, errors, and required authentication/confirmation instructions. Machine-mode stdout contains command results only; diagnostics use stderr without ANSI styling.
  • Route theme, help, and edit failures through the normal error path so machine-mode callers receive one structured error instead of an unexplained failure exit code.
  • Add analyzer CZ0003 to reject direct Console/AnsiConsole writes outside the output layer, with scoped exceptions for interactive line-editor drawing.
  • Preserve the concurrent import implementation already on main while migrating its diagnostic output.
  • Update command/navigation documentation, localized help, and the changelog.

Compatibility

Machine-mode watch now requires a positive --max and returns the collected changes as a bounded final result. Interactive mode still supports unbounded streaming until Ctrl+C. This intentional change is documented under Breaking changes.

Explicitly out of scope

No --mcp-stdio option or transport; no stdio startup buffering/timeouts, history lifecycle, authentication changes, command filtering, or MCP subscription/confirmation changes. HTTP MCP remains unchanged. This PR is based directly on main, not on the stdio branch, and does not modify #239.

Extract the output policy and command migrations from #239 without MCP stdio. Preserve diagnostics and required instructions in quiet mode, report command errors consistently, and require bounded machine-mode watch results.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-code-quality

github-code-quality Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Code coverage overview

Languages: C#

C# / code-coverage/dotnet

main 3acd215 dev/mkrueger/central... 1df4478 +/-
75% 75% 0%
View coverage of the 10 most impacted files.
File main 3acd215 dev/mkrueger/central... 1df4478 +/-
D:\a\CosmosDBSh...elcomeScreen.cs 96% 78% -18%
D:\a\CosmosDBSh...ThemeCommand.cs 71% 72% +1%
D:\a\CosmosDBSh...hell\Program.cs 83% 85% +2%
D:\a\CosmosDBSh...ucketCommand.cs 32% 34% +2%
D:\a\CosmosDBSh...\EditCommand.cs 56% 60% +4%
D:\a\CosmosDBSh...eItemCommand.cs 60% 79% +19%
D:\a\CosmosDBSh...atchExecutor.cs 17% 68% +51%
D:\a\CosmosDBSh...\ShellOutput.cs 0% 89% +89%
D:\a\CosmosDBSh...tputAnalyzer.cs 0% 90% +90%
D:\a\CosmosDBSh...eDiagnostics.cs 0% 100% +100%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated November 05, 2026 13:58 UTC

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Required editor prompts remain hidden in machine mode, and failed theme validation omits actionable diagnostics.

5 open findings
What changed in this PR

Centralizes CLI output policy while leaving HTTP MCP unchanged and deferring MCP stdio.

Changes:

  • Routes output through category-based handling for quiet and machine modes.
  • Standardizes command failures and requires bounded machine-mode watches.
  • Adds output enforcement, regression coverage, and updated documentation.
File Description
l10n/​CosmosDBShell.json Updates localized output guidance.
docs/​navigation.md Documents quiet and machine output.
docs/​commands.md Documents validation and bounded watches.
CosmosDBShell/​Program.cs Routes startup output centrally.
CosmosDBShell/​lang/​en.ftl Updates quiet and watch messages.
CosmosDBShell/​GlobalSuppressions.cs Removes obsolete output suppressions.
CosmosDBShell/​Azure.Data.Cosmos.Shell.KeyBindings/​ReverseSearchHistoryCommand.cs Exempts interactive editor drawing.
CosmosDBShell/​Azure.Data.Cosmos.Shell.KeyBindings/​ClearScreenCommand.cs Centralizes terminal clearing.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​ShellOutput.cs Implements centralized output handling.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​ShellInterpreter.cs Routes results, diagnostics, and prompts.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​OutputPolicy.cs Defines category presentation rules.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​CosmosCompletionRenderer.cs Exempts interactive completion drawing.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​WhoamiCommand.cs Routes identity presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​WatchCommand.cs Bounds machine-mode watches.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​UdfCommand.cs Categorizes function command output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​TtlCommand.cs Categorizes update messages.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​TriggerCommand.cs Categorizes trigger command output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ThroughputCommand.cs Routes results and confirmation instructions.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ThemeCommand.cs Standardizes failures and validation output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​SprocCommand.cs Categorizes stored-procedure output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​RmDbCommand.cs Routes deletion results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​RmContainerCommand.cs Routes deletion results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​RmCommand.cs Routes warnings and deletion results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ReplaceCommand.cs Controls replacement diagnostics.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​QueryCommand.cs Routes diagnostics and rejects invalid buckets.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​PwdCommand.cs Routes location presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​PatchCommand.cs Categorizes success messages.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​MakeItemCommand.cs Controls item-write diagnostics.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​MakeDbCommand.cs Routes creation results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​MakeContainerCommand.cs Routes creation results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ListCommand.cs Routes listing presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​InfoCommand.cs Routes statistics and settings presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​IndexCommand.cs Categorizes update messages.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ImportCommand.cs Migrates diagnostics without changing concurrency.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​HelpCommand.cs Routes help and throws lookup failures.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​FtabCommand.cs Routes table presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ExportCommand.cs Categorizes export summaries.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​EditCommand.cs Standardizes editor failures and output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​DoctorCommand.cs Routes diagnostic result presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​DisconnectCommand.cs Routes disconnection results.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​DirCommand.cs Routes directory presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ConnectCommand.cs Categorizes connection output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ConflictCommand.cs Categorizes update messages.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ClsCommand.cs Centralizes screen clearing.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​CdCommand.cs Categorizes navigation messages.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​CanICommand.cs Routes permission result presentation.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​BucketCommand.cs Standardizes invalid-bucket errors and output.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​BatchExecutor.cs Categorizes batch results and errors.
CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​BatchCommand.cs Routes batch presentation.
CosmosDBShell.Tests/​UtilTest/​ShellOutputTests.cs Tests output categories and routing.
CosmosDBShell.Tests/​Parser/​StatementPositionalErrorTests.cs Preserves returned-error location coverage.
CosmosDBShell.Tests/​Integration/​ShellProcessTests.cs Tests structured output and raw ANSI absence.
CosmosDBShell.Tests/​Integration/​AdditionalCommandsTests.cs Expects invalid-bucket failures.
CosmosDBShell.Tests/​CommandTests/​WatchCommandTests.cs Tests bounds and final-result emission.
CosmosDBShell.Tests/​CommandTests/​ThemeCommandTests.cs Expects validation exceptions.
CosmosDBShell.Tests/​CommandTests/​ThemeCommandDispatchTests.cs Expects theme dispatch exceptions.
CosmosDBShell.Tests/​CommandTests/​BucketCommandTests.cs Tests invalid-bucket exceptions.
CosmosDBShell.Analyzer/​GlobalSuppressions.cs Adds analyzer release-tracking suppression.
CosmosDBShell.Analyzer/​DirectConsoleOutputAnalyzer.cs Rejects direct console output.
CHANGELOG.md Records output changes and watch compatibility.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Address review feedback in #254 without changing singleton lifetime or cleanup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 12:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-cutting output routing and analyzer enforcement need human validation, with unresolved correctness issues remaining.

0 open findings

5 resolved since last review
Previously missed (2)

In code that hasn't changed since last review

Medium severity Enforce CZ0003 for Console.Clear()

CosmosDBShell.Analyzer/​DirectConsoleOutputAnalyzer.cs:76

Console.Clear() falls through this check without CZ0003. That allows terminal clearing outside ShellOutput.ClearScreen(), bypassing its machine-mode guard. Include Clear alongside Write* so the analyzer enforces the clearing policy as well.

Medium severity Record quiet MCP commands in editor history

CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​ShellInterpreter.cs:1914

When an editor is already running, this return skips Editor.History.Add(cmdString) for MCP commands executed with --quiet. The commands are recorded in shell history and saved when possible, but are missing from the editor's Up/Down history. Remove this guard and let the output policy suppress the echo without changing history behavior.

🧠 Review effort: Balanced

Address the previously missed overview findings in #254 and add focused analyzer and editor-history regression coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 13:05
@mkrueger

Copy link
Copy Markdown
Collaborator Author

Overview findings:

  • CZ0003 now also flags Console.Clear() outside ShellOutput.
  • Quiet command echoes are added to an existing editor's Up/Down history without being printed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-cutting output changes need human validation, and unresolved machine-mode error handling issues remain.

0 open findings

Previously missed (4)

In code that hasn't changed since last review

Medium severity Import diagnostics are lost in machine mode

CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ImportCommand.cs:668

This guard drops the import's per-item diagnostics in machine mode, including the line or item number, status, and service message. ExecuteAsync then throws only failure counts, so default -c callers cannot identify the failing input or distinguish a conflict from an authorization failure. Preserve these diagnostics through the import result and include them in the final structured error without changing concurrent scheduling.

Medium severity Bulk insert and upsert failure details are discarded

CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​MakeItemCommand.cs:150

The new machine-mode guards suppress bulk insert/upsert failure details as well as informational output. The final command-mkitem-error-array_failed exception contains only counts, so default -c callers lose the status and service message explaining a failure. Collect diagnostics from both failed-status responses and caught Cosmos exceptions, and include them in the final structured error.

Medium severity Bulk replacement errors are lost in machine mode

CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ReplaceCommand.cs:117

This guard discards each bulk replacement's error in machine mode, while the final exception reports only failure counts. A default -c replacement therefore loses actionable details such as a missing ID or service failure. Retain the per-item messages and include them in the final structured error, while keeping interactive diagnostics unchanged.

Medium severity Machine mode emits raw stack traces for output failures

CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​ShellInterpreter.cs:2037

In machine mode with --verbose, an output-formatting or file-redirection failure reaches WriteException, which now writes a raw multiline stack trace to stderr. This bypasses both the structured error contract and shell 2> redirection. For example, redirecting echo ok to a file in a missing directory triggers this path. Restrict the verbose renderer to non-machine mode so machine failures reach WriteMachineError below.

🧠 Review effort: Balanced

Address remaining overview findings in #254.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 13:24
@mkrueger

Copy link
Copy Markdown
Collaborator Author

Overview findings:

  • Import failures now retain source-line, status, and service-error details in the structured error.
  • Bulk insert/upsert and replace failures now include per-item diagnostics with 1-based item positions.
  • Machine-mode output failures remain structured with --verbose and honor 2> redirection.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-cutting output changes need human validation, and import diagnostic handling still has unresolved correctness and memory risks.

2 open findings

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-cutting output routing requires human review and build/test validation, alongside resolution of the retained correctness and resource-use concerns.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Bulk write diagnostics are unbounded and collected interactively

CosmosDBShell/​Azure.Data.Cosmos.Shell.Commands/​ReplaceCommand.cs:118

Bulk replacement retains every failure diagnostic without a count or length limit. MakeItemCommand does the same for inserts/upserts. Both also collect messages during interactive execution, where the lists are never consumed. Large failing arrays therefore retain all diagnostic text until completion, with additional copies when machine-mode errors are joined and serialized. Collect only in machine mode and bound both count and message length, reporting omitted diagnostics as the import path does. Reuse a shared bounded collector and cover both bulk-write paths.

Medium severity Shell stderr redirection does not capture validation warnings

CosmosDBShell/​Azure.Data.Cosmos.Shell.Core/​ShellOutput.cs:191

Shell 2>/2>> sets ErrOutRedirect and AppendErrRedirection; it does not replace Console.Error. Consequently, machine-mode theme validate warning.toml 2> warnings.log sends validation warnings to process stderr instead of the requested file. Query warnings and required instructions take the same path. Supply a redirection-aware stderr writer from the interpreter, preserving all writes within a command and the overwrite/append behavior. Add coverage for successful commands that emit warnings under both redirects.

2 resolved since last review

Review effort: Balanced

Cap per-item diagnostics for mkitem, replace, and import at 100 entries with an omitted count, skip collecting them in interactive mode, and route machine-mode ShellOutput stderr through 2>/2>> redirection.
Copilot AI balanced review requested due to automatic review settings October 19, 2026 10:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The batch error renderer can overwrite diagnostic details or duplicate errors when stderr redirection is active because it writes an error already reported by PrintStateCore.

1 open issue

Review effort: Balanced

Use the executing shell for batch renderers and avoid rendering an error a second time after PrintState reports it to stderr redirection.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to weigh an unpublished localization concern that can leave English prefixes in otherwise translated bulk-item errors.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Format insert, upsert, and replacement item positions through the existing localization catalog entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Fluent catalog contains a duplicate error-bulk-item definition that creates ambiguity for localization edits and should be removed.

1 open issue

Review effort: Balanced

Comment thread CosmosDBShell/lang/en.ftl Resolved
Copilot AI balanced review requested due to automatic review settings November 5, 2026 11:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because client-selection messages can use the singleton’s output policy instead of the executing shell’s, corrupting machine-mode stdout with human-readable text.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the centralized output-routing changes.

No open issues 🎉

Review effort: Balanced

Preserve both centralized-output and multi-statement process regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the centralized CLI output changes.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep centralized output and bounded diagnostics while preserving usage exit codes and returned-error reporting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Invalid option and argument values in query, watch, and bucket commands still produce exit code 1 instead of the documented usage-error code 2, preventing scripts from distinguishing input errors from operational failures.

3 open issues

Review effort: Balanced

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the centralized output-handling changes.

No open issues 🎉

3 resolved since last review

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants