Skip to content

[Dreaming] dreaming: clarify RenderImmediateMessage as the correct path for immediate TerminalLogger output#14381

Open
github-actions[bot] wants to merge 1 commit into
mainfrom
dreaming/terminal-logger-immediate-output-2835fe4d9a572fff
Open

[Dreaming] dreaming: clarify RenderImmediateMessage as the correct path for immediate TerminalLogger output#14381
github-actions[bot] wants to merge 1 commit into
mainfrom
dreaming/terminal-logger-immediate-output-2835fe4d9a572fff

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What changed

Added one bullet to .github/instructions/logging.instructions.md (Terminal Logger section):

Use RenderImmediateMessage (not direct terminal writes) for any message that must appear immediately outside the normal node-status rendering cycle — for example, messages with a null BuildEventContext or IsAuthProviderMessage events. Direct terminal writes bypass the node-display logic and corrupt the rendered output.

Why (recurring evidence)

Two independent PRs triggered the same correction from the same reviewer.

Duplication check

The existing Terminal Logger (FancyLogger) section in logging.instructions.md lists corruption concerns but does not mention RenderImmediateMessage. No other instruction or skill file covers this API contract.

Reviewers (core MSBuild team)

Suggested reviewers — core-team members who discussed this pattern (please assign): @rainersigwald

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Dreaming (learning atoms curation) · 86 AIC · ⌖ 6.17 AIC · ⊞ 8K ·

Recurring reviewer feedback (PRs #14356, #14370, rainersigwald) flagged
direct terminal writes for immediate messages instead of using the
established RenderImmediateMessage path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the Area: Documentation Issues about docs, including errors and areas we should extend (this repo and learn.microsoft.com) label Jul 15, 2026
@github-actions
github-actions Bot requested a review from a team as a code owner July 15, 2026 16:44
@github-actions github-actions Bot added the Area: Documentation Issues about docs, including errors and areas we should extend (this repo and learn.microsoft.com) label Jul 15, 2026
* Must handle terminal width changes, very narrow terminals, and non-TTY output (piped to file).
* Concurrent project builds must render without corruption.
* ANSI escape sequences must be cross-platform compatible.
* Use `RenderImmediateMessage` (not direct terminal writes) for any message that must appear immediately outside the normal node-status rendering cycle — for example, messages with a null `BuildEventContext` or `IsAuthProviderMessage` events. Direct terminal writes bypass the node-display logic and corrupt the rendered output.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* Use `RenderImmediateMessage` (not direct terminal writes) for any message that must appear immediately outside the normal node-status rendering cycle — for example, messages with a null `BuildEventContext` or `IsAuthProviderMessage` events. Direct terminal writes bypass the node-display logic and corrupt the rendered output.
* Use `RenderImmediateMessage` (not direct terminal writes) for any message that must appear immediately outside the normal node-status rendering cycle, like messages that require user intervention or explain long delays. Direct terminal writes bypass the node-display logic and corrupt the rendered output.

(null BEC is not an interesting part of the problem IMO).

ViktorHofer pushed a commit that referenced this pull request Jul 16, 2026
…itten member) (#14383)

## Context: validating the reviewers + multi-PR change

After #14373 merged, I triggered a live Dreaming run
([29432875878](https://github.com/dotnet/msbuild/actions/runs/29432875878))
to validate the new behavior. Results:

- ✅ **Multi-PR worked** — the run opened **two atomic PRs**, each
touching a single file with a distinct theme (#14381 logging
instructions, #14382 expert-reviewer agent). Exactly the intended
one-pattern-per-PR shape.
- ✅ **Reviewer section rendered** on both, with the softened "please
assign" wording.
- ❌ **One reviewer was wrong.** #14382 listed `@baronfel`, who is
**not** on the `@dotnet/kitten` roster (`gh api
orgs/dotnet/teams/kitten/memberships/baronfel` → 404) **and** was not in
that PR's cited discussion (only `@jankratochvilcz` was). The agent
invented a plausible-sounding name instead of strictly filtering.
(#14381's `@rainersigwald` was correct — active kitten member and the
actual reviewer.)

## Fix

Tighten Step 5b so a reviewer qualifies **only if the login is in
BOTH**:
1. the fetched `orgs/dotnet/teams/kitten/members` output, **and**
2. that pattern's actual discussion-participant list.

Plus explicit guardrails:
- **Never** add a name from memory / general knowledge of "who works on
MSBuild" / because it seems plausible.
- A **pre-emit self-check**: before emitting each PR, verify every
`@`-mention on the reviewers line is in both lists; drop any that fail.
- When in doubt, prefer the `@dotnet/kitten` fallback over guessing an
individual.

## Scope

Only the Dreaming workflow prompt + recompiled lock (1-line lock change:
the embedded prompt hash/text). Compiles clean (0/0). This directly
closes the gap the validation run exposed.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jankratochvilcz <2202487+jankratochvilcz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Documentation Issues about docs, including errors and areas we should extend (this repo and learn.microsoft.com)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants