Skip to content

Sync upstream — 15 commits from microsoft/terminal (through #20260)#220

Merged
yeelam-gordon merged 17 commits into
mainfrom
upstream-sync/2026-06-04
Jun 4, 2026
Merged

Sync upstream — 15 commits from microsoft/terminal (through #20260)#220
yeelam-gordon merged 17 commits into
mainfrom
upstream-sync/2026-06-04

Conversation

@yeelam-gordon

Copy link
Copy Markdown
Collaborator

⚠️ MERGE METHOD: REBASE-MERGE OR MERGE-COMMIT — NOT SQUASH.
Squash destroys the per-commit author/committer/date metadata this PR was carefully built to preserve.
Auto-merge is armed with --rebase.

First real run of the upstream-sync skill (introduced in #218).
Brings 15 commits from microsoft/terminal/main into this fork, each one preserving its original author, committer, and dates (incl. timezone offsets).

Range

  • From a325a2fa5 (merge-base on 2026-06-04, ""Fix settings error text legibility in High Contrast mode #20098"")
  • To 5e912f5df (#20260 — chore: remove remaining references to ATL and MFC)
  • Picked: 15 of 16 pending commits

What's in this PR

# SHA Author PR Subject
1 8033c56 Console Service Bot #20168 Localization Updates - main - 04/30/2026
2 ba29891 Console Service Bot #20196 Localization Updates - main - 05/07/2026
3 afb1ac8 Dustin L. Howett #20183 Reject DTTERM Window Manipulation resizes with the current size
4 b19913d Dustin L. Howett #20214 Only set startingTitle once, clear up title fallback handling
5 2ccdfb4 Dustin L. Howett #20213 sixel: prevent allocating an absurd amount of memory or writing OOB
6 82e24ab Lucas Trzesniewski #20207 Add safeUriSchemes setting [1 conflict auto-resolved]
7 23db671 aarushi singh #20032 Make DECSTR cursor restore behave like xterm in alt screen buffer
8 b9b1d50 aarushi singh #20031 Use PlaySoundW for profile bell sounds
9 0c5543c Dustin L. Howett #20230 Keep the font size delta across settings reloads
10 335c03c Carlos Zamora #20203 Redesign new tab menu page -> dropdown
11 2616193 Mike Griese #20256 Preview the selected tab in tabSearch too
12 304adb6 Dustin L. Howett #20271 meta: add a solution filter that only opens conhost components
13 8f8c63b Carlos Zamora #20275 Add screen reader announcement for opening/closing navigation pane
14 b33f64b Console Service Bot #20259 Localization Updates - ""Dropdown Menu""
15 5e912f5 Dustin L. Howett #20260 chore: remove remaining references to ATL and MFC [1 conflict resolved take-upstream]

Conflicts resolved during cherry-pick

  1. safeUriSchemes (#20207)src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl
    Add-add conflict: fork added several agent settings (AcpAgent, DelegateAgent, etc.) and upstream added SafeUriSchemes after the same anchor line. Resolution: union (keep both). SafeUriSchemes inserted right after AutoFixEnabled and before the agent-specific block. The .cpp, .h, and other files merged cleanly.

  2. ATL/MFC cleanup (#20260)src/common.build.pre.props
    Upstream removed warning suppressions 4467 and 4459 (no longer needed after ATL/MFC removal). Fork hadn't touched these. Resolution: take-upstream.

What's NOT in this PR

93bdbfaa3 — Add support for OSC777 (Send Notification) (#20012)deferred to a follow-up PR.

OSC777 adds an IShowNotification callback chain (Terminal → ITerminalApi → Core → TermControl → UI) at the exact same call sites in 11 files where the fork's autofix pipeline added its own VtSequenceReceived chain. The two features are independent and should coexist — but the merge requires careful method-by-method co-installation (not blind union — adapterTest.cpp has different mock bodies), which is a real human-review call.

Conflicted files for the OSC777 follow-up:
ControlCore.{cpp,h,idl}, TermControl.{cpp,h,idl}, Terminal.{cpp,hpp}, TerminalApi.cpp, outputStream.cpp, ITerminalApi.hpp, adapterTest.cpp.

Verification

  • ✅ All 15 commits show original %an %ae %aI and %cn %ce %cI (verified via git log --format after push)
  • ✅ Timezone offsets preserved (-05:00, -07:00, +02:00, Z)
  • state.json will be updated to last_synced_upstream_sha=5e912f5df once OSC777 follow-up lands (so the next run resumes from after OSC777)

Skill demo notes

This run validated the skill's design end-to-end:

  • ✅ Cherry-pick loop with env-var author/committer preservation worked
  • ✅ Tier-2 add-add conflicts can be hand-resolved as union without losing fork code
  • ✅ Take-upstream resolution worked for the build-props warning list
  • ✅ Tier-3 semantic conflict (OSC777 vs autofix hooks) correctly halted the run instead of producing a broken merge

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

consvc and others added 15 commits May 6, 2026 16:36
Co-authored-by: Console Service Bot <consvc@microsoft.com>
(cherry picked from commit ea4cb8145f768c49e5b91229eb77f5ad1135f9ad)
Co-authored-by: Console Service Bot <consvc@microsoft.com>
(cherry picked from commit d3f76e7acf88436497850ce36ca4fa5c8bb61be0)
This may help #20182 and #20112
Closes #19310

(cherry picked from commit 3e3b3ad883d134bdab26850d42e2a8308da40086)
This commit ensures that we only set the starting title once when we
open a new terminal pane.

It also consolidates all title selection into Terminal::GetConsoleTitle,
which is now used in the TitleChanged event.

TitleChanged no longer stores a separate copy of the starting title if
an application attempts to _clear_ the title; that seems like a poorer
implementation of what we already had.

This supersedes work in #20204.

Closes #19340
Closes #20204

Co-authored-by: imsh <im.shaedar@gmail.com>
(cherry picked from commit b991eb048ed2ce8de40d551a217fbf32351b200a)
…#20213)

This commit implements two fixes for the integer overflow/out-of-bounds
write reported in #20149.

First, it catches any exception generated in sixel char processing
(which will prevent `out_of_memory` or `bad_alloc` from being ignored
sight-unseen, and will prevent the consumption of further DCS content).

Second, it prevents us from allocating memory for an image which will
never be displayed (because it exceeds the height of the display.)

This supersedes prior work in #20153 for the same issues.

Closes #20149
Closes #20153

Co-authored-by: James Holderness <j4_james@hotmail.com>
(cherry picked from commit c829d4ca548337b515e7f16a6743fda532e013f4)
This adds a `safeUriSchemes` global setting which lets you define
hyperlink URI schemes which the user considers safe. No confirmation
dialog will be shown when trying to open hyperlinks which use these
schemes.

- This solves the root issue, but doesn't introduce any UI or
  documentation changes. I wanted to validate the approach and
  implementation with you first.
- I closely followed the code handling the `disabledProfileSources`
  setting, which is of the same type.
- This feature does not change the behavior of `http`, `https` and
  `file` schemes.

Validation

I ran the dev terminal, and tested the behavior by clicking on `vscode`
hyperlinks generated by ripgrep with various `safeUriSchemes` settings:

- Setting not defined - asks for confirmation
- `["vscode"]` - does not ask for confirmation
- `["foo", "vscode"]` - does not ask for confirmation
- `["foo"]` - asks for confirmation
- `null` - asks for confirmation
- `[]` - asks for confirmation
- `[""]` - asks for confirmation
- `[{"foo": "bar"}]` - fails to deserialize (as expected)

A few uinit tests failed, but they seem unrelated to these changes:
- `KeyBindingTests` in `UnitTests_SettingsModel`, probably because I use
  an AZERTY keyboard.
- A few `Conhost` tests, but I didn't touch this part

Refs #20065
Closes #20191
…0032)

There is some disagreement among terminal emulators as to whether DECSTR
(and therefore, soft reset) restores the cursor in the active buffer or
in all buffers.

We had previously chosen to restore the cursor in all buffers.

xterm restores the cursor only in the active buffer.

Closes #19918

(cherry picked from commit 12e3455bb2bd62745fad87e1924ec8ef115ec54a)
We believed that this would fix an issue on Windows 10, where the volume
mixer would forget Windows Terminal after every relaunch. It turns out
that it does not.

Still, this code is much more concise and doesn't require yet another
WinRT object. Story of our lives.

Refs #17733

(cherry picked from commit abeac1b1355d6ddd23ebdda9c1e4ddab6fcaac14)
This is a trivial fix for an issue we get a somewhat outsized number of
complaints about.

When the user has adjusted the font size in any direction, we'll just
keep track of the magnitude and apply it every time the settings change.

Yes, this means that if you zoom once and then change your real font
size it's going to zoom even more.

That probably doesn't matter.

Refs #11522

(cherry picked from commit 8fe6c21ef88a73a7985b5968ee18936928ccac69)
## Summary of the Pull Request
Renames the "New tab menu" page to "Dropdown menu".
Redesigns the page with the following notes:
- the preview is now on the right side so that it's always visible
- moves preview buttons into border with previewed new tab menu entries
- when the window narrows, removes the icons from the controls, then
changes layout to be vertical so that it all fits.

Aside from the main redesign changes above, a lot of the other changes
are to make sure that spacing and alignment looks ok.

## Validation Steps Performed
- [x] Wide view
- [x] Medium width view
- [x] Narrow view

## PR Checklist
Related to #17000

(cherry picked from commit a8a5c881119fdc4c85126cd1cb48945365dfc240)
Currently when you use the cmdpal in tabswitcher mode, we "preview"
switching to that tab. We don't do that for `tabSearch` mode though.

Fixing that was half a line of code. Now we do.

Closes #20233

(cherry picked from commit 458872a01a2d8d37a43ee5c525bbcf061cb23d54)
It loads faster, and a full "solution" build produces everything
required to run and test conhost.

(cherry picked from commit 8ef8e95eb54bc93e2ee47ceb9c39ce46d286c3d9)
…20275)

## Summary of the Pull Request
Adds a manual event handler for `PaneOpened`/`PaneClosed` on the
navigation view in the settings UI. After catching the event, we raise a
UIA notification event to force the attached screen reader to read out
"Navigation pane opened/closed" appropriately. This experience aligns
with WinUI and the text is localized appropriately.

## Validation Steps Performed
✅ Narrator announces "Navigation pane opened/closed" appropriately
(aligns with WinUI 3 gallery experience)

## PR Checklist
Closes #19687
📝 NOTE: the associated issue is currently affecting our accessibility
score

(cherry picked from commit 00e0ecb8b50e45137a6496a3e05028124b4ea7e5)
Co-authored-by: Console Service Bot <consvc@microsoft.com>
(cherry picked from commit 7fc12cf949a148277dd6a7fdbec3d0302b125570)
Apparently we got rid of ATL in 2019 with #719

(cherry picked from commit 7d582e9290a958144fa11c7149f5e6c9b2b9e05e)
Copilot AI review requested due to automatic review settings June 4, 2026 06:29
@yeelam-gordon yeelam-gordon enabled auto-merge (rebase) June 4, 2026 06:29

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.

Pull request overview

This PR syncs 15 upstream commits from microsoft/terminal into this fork (through upstream PR #20260), preserving original author/committer metadata, and includes two manual conflict resolutions (safeUriSchemes union in GlobalAppSettings.idl, and taking upstream for the ATL/MFC warning suppression cleanup).

Changes:

  • Adds a new global setting safeUriSchemes (schema + settings model + app hyperlink safety check + serialization test).
  • Pulls in multiple upstream behavior/robustness updates (sixel parsing hardening, DECSTR cursor restore behavior, title handling tweaks, font zoom delta persistence, tab search preview).
  • Brings in UI/accessibility + localization updates (settings editor dropdown-menu redesign, navigation pane announcements, new/updated localized strings), plus conhost-only solution filter and removal of remaining ATL/MFC references.

Reviewed changes

Copilot reviewed 69 out of 69 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/unit.tests.x86.runsettings Removes ATL-related coverage exclusions following ATL/MFC cleanup.
src/unit.tests.x64.runsettings Removes ATL-related coverage exclusions following ATL/MFC cleanup.
src/terminal/adapter/SixelParser.cpp Adds exception guard during sixel parsing and limits buffer growth when content goes beyond display.
src/terminal/adapter/adaptDispatch.cpp Updates DECSTR soft reset cursor restore behavior to match xterm in alt buffer cases.
src/host/ut_host/ScreenBufferTests.cpp Adds a unit test validating DECSTR behavior when executed in the alt buffer.
src/common.build.pre.props Drops ATL/MFC-related warning suppressions (C4467/C4459).
src/cascadia/UnitTests_SettingsModel/SerializationTests.cpp Adds safeUriSchemes to serialization coverage.
src/cascadia/TerminalSettingsModel/Resources/zh-TW/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/zh-CN/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/ru-RU/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/qps-plocm/Resources.resw Updates pseudo-locale “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/qps-ploca/Resources.resw Updates pseudo-locale “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/qps-ploc/Resources.resw Updates pseudo-locale “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/pt-BR/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/ko-KR/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/ja-JP/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/it-IT/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/fr-FR/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/es-ES/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/Resources/de-DE/Resources.resw Updates localized “restart connection/session” text.
src/cascadia/TerminalSettingsModel/MTSMSettings.h Adds SafeUriSchemes to the global settings X-macro list.
src/cascadia/TerminalSettingsModel/GlobalAppSettings.idl Adds SafeUriSchemes property to the settings model IDL (conflict resolution).
src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp Ensures SafeUriSchemes is copied in GlobalAppSettings::Copy().
src/cascadia/TerminalSettingsEditor/SettingContainerStyle.xaml Refactors default SettingContainer style into a keyed style + implicit alias.
src/cascadia/TerminalSettingsEditor/Resources/zh-TW/Resources.resw Adds/updates localized strings for new UX items (confirm-on-close dropdown, dropdown menu rename, etc.).
src/cascadia/TerminalSettingsEditor/Resources/zh-CN/Resources.resw Adds/updates localized strings for new UX items (confirm-on-close dropdown, dropdown menu rename, etc.).
src/cascadia/TerminalSettingsEditor/Resources/ru-RU/Resources.resw Adds/updates localized strings for new UX items (confirm-on-close dropdown, dropdown menu rename, etc.).
src/cascadia/TerminalSettingsEditor/Resources/qps-plocm/Resources.resw Adds/updates pseudo-locale strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/qps-ploca/Resources.resw Adds/updates pseudo-locale strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/qps-ploc/Resources.resw Adds/updates pseudo-locale strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/pt-BR/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/ko-KR/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/ja-JP/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/it-IT/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/fr-FR/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/es-ES/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw Adds screen reader announcement strings + renames “New Tab Menu” to “Dropdown Menu”.
src/cascadia/TerminalSettingsEditor/Resources/de-DE/Resources.resw Adds/updates localized strings for new UX items.
src/cascadia/TerminalSettingsEditor/NewTabMenu.xaml Redesigns the page layout to a controls+preview responsive grid with visual states.
src/cascadia/TerminalSettingsEditor/MainPage.xaml Hooks PaneOpened/PaneClosed for navigation view.
src/cascadia/TerminalSettingsEditor/MainPage.h Declares pane open/close handlers + helper announcement method.
src/cascadia/TerminalSettingsEditor/MainPage.cpp Raises UIA notification events when the settings nav pane opens/closes.
src/cascadia/TerminalCore/terminalrenderdata.cpp Updates title fallback to return empty when no title is available.
src/cascadia/TerminalCore/TerminalApi.cpp Adjusts title-setting semantics and avoids no-op resize notifications.
src/cascadia/TerminalCore/Terminal.hpp Makes _startingTitle optional to support “set once” semantics.
src/cascadia/TerminalCore/Terminal.cpp Sets _startingTitle only once across settings reloads.
src/cascadia/TerminalControl/ControlCore.h Adds _accumulatedFontSizeDelta to preserve zoom across reloads.
src/cascadia/TerminalControl/ControlCore.cpp Applies accumulated font delta during settings reload and refactors reset/adjust logic.
src/cascadia/TerminalApp/TerminalPaneContent.h Removes MediaPlayer-based bell playback members and coroutine signature.
src/cascadia/TerminalApp/TerminalPaneContent.cpp Switches bell playback to PlaySoundW and removes MediaPlayer cleanup/coroutine.
src/cascadia/TerminalApp/TerminalPage.h Makes URI “somewhat safe” check an instance method (to use settings).
src/cascadia/TerminalApp/TerminalPage.cpp Uses safeUriSchemes to allow additional schemes without confirmation.
src/cascadia/TerminalApp/Resources/zh-TW/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/zh-CN/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/ru-RU/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/qps-plocm/Resources.resw Updates pseudo-locale versions of new/changed strings.
src/cascadia/TerminalApp/Resources/qps-ploca/Resources.resw Updates pseudo-locale versions of new/changed strings.
src/cascadia/TerminalApp/Resources/qps-ploc/Resources.resw Updates pseudo-locale versions of new/changed strings.
src/cascadia/TerminalApp/Resources/pt-BR/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/ko-KR/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/ja-JP/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/it-IT/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/fr-FR/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/es-ES/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/Resources/de-DE/Resources.resw Updates confirm-close dialog strings, notification strings, and restart-session strings.
src/cascadia/TerminalApp/CommandPalette.cpp Extends “switch to tab” behavior to tab search mode.
doc/cascadia/profiles.schema.json Documents safeUriSchemes in the settings schema.
conhost.slnf Adds a solution filter focusing on conhost-related projects.
.github/actions/spelling/expect/expect.txt Adds spelling “expect” entries for new identifiers/extensions.

Comment thread src/cascadia/TerminalControl/ControlCore.cpp
Comment thread src/terminal/adapter/SixelParser.cpp
Comment thread .github/actions/spelling/expect/expect.txt
Comment thread .github/actions/spelling/expect/expect.txt
Comment thread .github/actions/spelling/expect/expect.txt
Comment thread src/cascadia/TerminalApp/Resources/zh-TW/Resources.resw
Comment thread src/cascadia/TerminalSettingsModel/Resources/zh-TW/Resources.resw
yeelam-gordon added a commit that referenced this pull request Jun 4, 2026
…kill

Closes the PR #220 audit gap: clean cherry-picks were producing broken PRs because git-level conflict detection missed content-level issues (duplicate .resw keys, dropped fork-specific warning suppressions).

New gates run AFTER the cherry-pick loop, BEFORE push/PR:
  1. Toolchain preflight  -> detect missing PlatformToolset (infra)
  2. Static breakage scan -> resw dup baseline-diff + fork invariants
  3. Try-build            -> razzle + bz no_clean (45min timeout)

Failures become Tier-4 stuck (post-pick validation), distinct from Tier-3 cherry-pick conflicts. State schema extended with a new stuck_validation field alongside the existing stuck_on_sha; either being set causes the scheduler to skip.

Smoke-tested 08-static-scan against the known-bad PR #220 worktree: returns 26 critical resw-duplicate findings + 1 high C4459 invariant finding, blocking=true (matches the PR #220 audit exactly). Toolchain preflight verified to detect missing v143 on a v145-only host. 10-try-build is wired but not end-to-end tested (would take 30+ min).

Also fixed:
- gh now uses -R microsoft/intelligent-terminal explicitly on issue/pr calls (the upstream remote was making gh default to microsoft/terminal).
- workflow.md: --tags=false -> --no-tags (invalid flag).

v1 scope: resw duplicate keys (baseline-aware) + fork invariants from references/fork-invariants.json (seed: C4459 suppression). v2 deferred: missing-include / vcxproj drift (try-build catches these with zero false positives, so not worth a separate static check).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yeelam-gordon and others added 2 commits June 4, 2026 15:41
Two content-level issues slipped past the clean git-level cherry-pick (now caught by the new static-scan in PR #218):

1. Duplicate <data name> entries in 20 .resw files (10 locales x 2 projects, 210 blocks total). Root cause: a fork-local commit (ddf97ab) appended ConfirmCloseDialog_* / NotificationMessage_TabActivity* keys; upstream commit b753e3d then renamed the old QuitDialog_* keys to the same names in-place. Resolution: drop the fork-appended duplicates, keep upstream's in-place positions (which preserves alphabetical ordering).

2. C4459 warning suppression dropped from src/common.build.pre.props. Root cause: upstream removed unrelated C4467 in the same DisableSpecificWarnings list during the ATL/MFC cleanup, and the take-upstream resolution wholesale lost the fork-specific C4459 too. C4459 (declaration of 'X' hides class member) is required because the fork builds with TreatWarningsAsError=true and fork-specific code triggers it. Restored.

Verified with .github/skills/upstream-sync/scripts/08-static-scan.ps1: blocking=false, 0 critical, 0 high.

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

PR #220's first dedup pass (fd8332b) used a regex requiring CRLF after </data>,
so it cleaned the 20 multi-line-formatted locale files but skipped the 3 pseudo-
locales (qps-ploc, qps-ploca, qps-plocm) where upstream localization appends
fork-only AIAgents_* entries as a single concatenated <data>…</data><data>…</data>
line. That line carries duplicates of entries already present in multi-line form
earlier in the same file.

Build failed with:
  PRI175: Duplicate Entry / PRI277: Conflicting values for resource
  'Microsoft.Terminal.Settings.Editor/Resources/Globals_ConfirmOnCloseAutomatic/Content'

This commit reruns the dedup with a CRLF-agnostic regex so the inline form is
also collapsed. 63 duplicate <data> blocks removed across 6 files (15 each in
TerminalApp pseudo-locales, 6 each in TerminalSettingsEditor pseudo-locales).
First occurrence per name is preserved; CRLF + UTF-8 BOM preserved.

Verification: post-dedup scan of all 40 changed resw files against origin/main
reports 0 newly-duplicated <data name> entries.

Follow-up: the upstream-sync skill's 08-static-scan.ps1 missed both this and
the original 26 critical findings because Get-FileTextOnDisk uses
[System.IO.File]::ReadAllText with a relative path, which resolves against
.NET's [Environment]::CurrentDirectory (not PowerShell's $PWD). Will be
fixed separately on dev/yeelam/upstream-sync-skill.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 4, 2026 08:04
yeelam-gordon added a commit that referenced this pull request Jun 4, 2026
… audit miss)

Get-FileTextOnDisk used [System.IO.File]::ReadAllText with a relative path.
.NET resolves relative paths against [System.Environment]::CurrentDirectory,
NOT PowerShell's $PWD, so the scan was silently reading from
`Q:\official\intelligent-terminal` (the unrelated main worktree) while
the orchestrator was cd'd into `Q:\official\it-sync` (the sync worktree
with the duplicates).

PR #220 symptom: the scan returned �locking=false / 0 critical even
though the cherry-pick batch had introduced 63 new duplicate `<data
name>` entries across 6 pseudo-locale files, and the build subsequently
failed with PRI175 / PRI277.

Fix:
- Get-FileTextOnDisk: resolve to absolute via Join-Path (Get-RepoRoot)
  when the input is relative.
- Get-FileTextAtRef: capture `git show` output through a temp file via
  `cmd /c "git show ref:path > tmp 2>nul"` instead of PowerShell's
  pipeline, which truncates/mangles high-Unicode pseudo-locale bytes
  when the subprocess's stdout binds to PSObject string formatting.

Verified against PR #220 commits:
- HEAD = fd8332b (the buggy state): 6 critical findings, blocking=true ✅
- HEAD = d6b4dc3 (the fixed state): 0 critical, blocking=false ✅

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.

Pull request overview

Copilot reviewed 69 out of 69 changed files in this pull request and generated 1 comment.

Comment thread src/terminal/adapter/SixelParser.cpp
@yeelam-gordon yeelam-gordon merged commit 75f8a84 into main Jun 4, 2026
11 checks passed
yeelam-gordon added a commit that referenced this pull request Jun 4, 2026
Two content-level issues slipped past the clean git-level cherry-pick (now caught by the new static-scan in PR #218):

1. Duplicate <data name> entries in 20 .resw files (10 locales x 2 projects, 210 blocks total). Root cause: a fork-local commit (ddf97ab) appended ConfirmCloseDialog_* / NotificationMessage_TabActivity* keys; upstream commit b753e3d then renamed the old QuitDialog_* keys to the same names in-place. Resolution: drop the fork-appended duplicates, keep upstream's in-place positions (which preserves alphabetical ordering).

2. C4459 warning suppression dropped from src/common.build.pre.props. Root cause: upstream removed unrelated C4467 in the same DisableSpecificWarnings list during the ATL/MFC cleanup, and the take-upstream resolution wholesale lost the fork-specific C4459 too. C4459 (declaration of 'X' hides class member) is required because the fork builds with TreatWarningsAsError=true and fork-specific code triggers it. Restored.

Verified with .github/skills/upstream-sync/scripts/08-static-scan.ps1: blocking=false, 0 critical, 0 high.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yeelam-gordon added a commit that referenced this pull request Jun 4, 2026
Codifies the fix-in-PR vs. follow-up-PR policy learned on PR #220 / #226:
build-blocking failures get one extra commit on the sync branch, but all
substantive review feedback (Copilot or human) goes into a separate
follow-up PR based on the sync branch's HEAD. Preserves the cherry-pick
PR's 'faithful to upstream' audit property.

- SKILL.md: new 'After-PR review handling' subsection + Gotcha bullet.
- references/follow-up-pr.md: new doc with full rubric, worktree
  mechanics, reply+resolve flow, and rebase-onto-main fallback.
- references/workflow.md: new step 10 cross-linking the policy.
- scripts/06-finalize-pr.ps1: PR banner now spells the policy out so the
  first reviewer doesn't push back on deferred fixes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

9 participants