Fix breadcrumb bridge keyboard-navigation defects: Enter-key binding, scroll-into-view, and #440 router-test coverage gap

UtilitiesCS.Test/OutlookObjects/Folder/BreadcrumbHtmlRendererTests.cs

@@ -93,6 +93,26 @@ public void Issue439ActiveAncestorChildrenAndEmbeddedBridgeUseTypedStoppedActiva
             document.Should().NotContain("ItemViewer");
         }
 
+        /// <summary>
+        /// Issue #737 Findings 1 and 2: string-containment assertion against the public
+        /// <see cref="BreadcrumbDocumentAssets.BridgeJs"/> constant, following the
+        /// <see cref="Issue439ActiveAncestorChildrenAndEmbeddedBridgeUseTypedStoppedActivation"/>
+        /// precedent. This repository's test suite has no headless-browser or JS-engine
+        /// dependency, so this test verifies the JS text is present and correctly shaped --
+        /// not that it executes correctly in a real WebView2/Chromium document.
+        /// </summary>
+        [TestMethod]
+        public void Issue737BridgeJsPostsRowSelectedOnEnterAndScrollsSelectedRowIntoView()
+        {
+            // Act
+            string bridgeJs = BreadcrumbDocumentAssets.BridgeJs;
+
+            // Assert
+            bridgeJs.Should().Contain("e.key === 'Enter'");
+            bridgeJs.Should().Contain("post({ type: 'rowSelected', rowId: id });");
+            bridgeJs.Should().Contain("scrollIntoView({ block: 'nearest' })");
+        }
+
         [TestMethod]
         public void RenderRowFragment_EveryRowKind_EmitsTrailingPctFlexItem()
         {

UtilitiesCS.Test/OutlookObjects/Folder/FolderBreadcrumbBridgeRouterTests.cs

@@ -371,13 +371,17 @@ public async Task Route_LeftArrow_NothingToCollapse_ReportsUnhandledLeft()
             // presses are needed to reach the root. Only once the root is active does nothing
             // remain to collapse and no further tree transition apply.
             var router = await PopulatedRouterAsync(ProviderMock());
-            await ArrowAsync(router, "left");
-            await ArrowAsync(router, "left");
+            var firstPress = await ArrowAsync(router, "left");
+            var secondPress = await ArrowAsync(router, "left");
 
             // Act
             var outputs = await ArrowAsync(router, "left");
 
             // Assert
+            firstPress.Should().ContainSingle();
+            BreadcrumbBridgeSerializer.Parse(firstPress[0]).Should().BeOfType<RenderMessage>();
+            secondPress.Should().ContainSingle();
+            BreadcrumbBridgeSerializer.Parse(secondPress[0]).Should().BeOfType<RenderMessage>();
             outputs.Should().ContainSingle();
             ((UnhandledArrowMessage)BreadcrumbBridgeSerializer.Parse(outputs[0]))
                 .Direction.Should()

UtilitiesCS/OutlookObjects/Folder/BreadcrumbDocumentAssets.cs

@@ -99,6 +99,13 @@ public static class BreadcrumbDocumentAssets
             + "    }\n"
             + "  });\n"
             + "  document.addEventListener('keydown', function (e) {\n"
+            + "    if (e.key === 'Enter') {\n"
+            + "      var selected = document.querySelector('.rowwrap.selected');\n"
+            + "      var id = selected ? selected.getAttribute('data-row-id') : '';\n"
+            + "      post({ type: 'rowSelected', rowId: id });\n"
+            + "      e.preventDefault();\n"
+            + "      return;\n"
+            + "    }\n"
             + "    var map = { ArrowLeft: 'Left', ArrowRight: 'Right', ArrowUp: 'Up', ArrowDown: 'Down' };\n"
             + "    var key = map[e.key];\n"
             + "    if (!key) { return; }\n"
@@ -131,6 +138,8 @@ public static class BreadcrumbDocumentAssets
             + "          host.appendChild(div);\n"
             + "        });\n"
             + "      }\n"
+            + "      var scrollTarget = document.querySelector('.rowwrap.selected');\n"
+            + "      if (scrollTarget) { scrollTarget.scrollIntoView({ block: 'nearest' }); }\n"
             + "    });\n"
             + "  }\n"
             + "})();\n";

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/code-review.2026-09-03T02-30.md

@@ -0,0 +1,93 @@
+# Code Review — breadcrumb-bridge-keyboard-navigation-defects (Issue #737)
+
+- Timestamp: 2026-09-03T02-30
+- Scope: full branch diff `origin/main...HEAD` (three-dot, base = `b13d5b7b`, the confirmed merge-base) — 3 code files (1 production, 2 test), plus feature-folder docs/evidence reviewed for consistency only.
+- Files reviewed in depth: `UtilitiesCS/OutlookObjects/Folder/BreadcrumbDocumentAssets.cs`, `UtilitiesCS.Test/OutlookObjects/Folder/FolderBreadcrumbBridgeRouterTests.cs`, `UtilitiesCS.Test/OutlookObjects/Folder/BreadcrumbHtmlRendererTests.cs`.
+
+## Summary
+
+This is a small, well-scoped bug-fix branch delivering two additive JavaScript-content changes (Enter-key binding, post-render scroll-into-view) to an inline JS string constant, plus one test-quality fix (capturing two previously-discarded Arrange-phase results in an existing router test). No production C# logic changed; no new C#-side types, message shapes, or router branches. All three changed files match the spec's designed diffs line-for-line. No blocking findings.
+
+## Finding 1 (#640) — Scroll-into-view — `BreadcrumbDocumentAssets.cs`
+
+```
++ "      var scrollTarget = document.querySelector('.rowwrap.selected');\n"
++ "      if (scrollTarget) { scrollTarget.scrollIntoView({ block: 'nearest' }); }\n"
+```
+
+Placement: these two lines sit after the `if (msg.type === 'render') {...} else if (msg.type === 'subfolderResult') {...}` block and before the closing `});` of the `addEventListener('message', ...)` callback (lines 141-142 of the current file). This means the scroll-into-view call runs unconditionally at the end of every inbound `message` event, not gated to only the `render`/`subfolderResult` branches specifically. Functionally this is equivalent today, because those are the only two `msg.type` values the listener currently recognizes (confirmed by reading the full `BridgeJs` constant — no other `if`/`else if` arm exists in this listener). This is a minor design note, not a defect: if a third inbound message type is ever added to this listener without updating this comment, the scroll call would silently also fire for it. Non-blocking; worth a one-line comment if a maintainer wants to make the intent explicit in a follow-up, but not required by the spec or the ACs.
+
+## Finding 2 (#641) — Enter-key binding — `BreadcrumbDocumentAssets.cs`
+
+```
++ "    if (e.key === 'Enter') {\n"
++ "      var selected = document.querySelector('.rowwrap.selected');\n"
++ "      var id = selected ? selected.getAttribute('data-row-id') : '';\n"
++ "      post({ type: 'rowSelected', rowId: id });\n"
++ "      e.preventDefault();\n"
++ "      return;\n"
++ "    }\n"
+```
+
+- Matches spec.md's Design section verbatim (reuses `rowSelected`, no new message type/codec branch/router case — independently confirmed by reading `QuickFiler/Controllers/BreadcrumbBridgeRouter.cs` and finding the pre-existing `case BreadcrumbMessageTypes.RowSelected: SelectRow(row); break;` at line 285-287, unmodified).
+- **Duplication observation (minor, non-blocking)**: the 3-line `.rowwrap.selected` -> `getAttribute('data-row-id')` -> ternary-to-`''` pattern in this new Enter branch is byte-identical to the pre-existing arrow-key branch's lookup at lines 112-114 (`var selected = document.querySelector('.rowwrap.selected'); var id = selected ? selected.getAttribute('data-row-id') : ''; post({ type: 'arrowKey', ...`). This is copy-paste duplication that a small local JS helper function could remove. It is explicitly spec-mandated, though: spec.md's Design section for Finding 2 instructs "using the identical `document.querySelector('.rowwrap.selected')` lookup the arrow-key handler already performs," i.e., the duplication is an intentional mirror of existing style rather than a new pattern introduced by this change. Given the file has no JS test-execution harness and the string-literal-concatenation authoring style makes refactoring into a shared JS function mechanically awkward within a C# string constant, this is reasonable to leave as-is; flagged for awareness only.
+- **Edge-case safety (verified, not merely assumed)**: when Enter is pressed with no `.rowwrap.selected` element present, `id` resolves to `''`. On the C# side, `ProcessInboundAsync` resolves the row via `FindRow(message.RowId)` before dispatching to any `case` (line 236); an empty/unmatched `RowId` returns `null`, which is logged and the message is dropped without dispatching to `SelectRow` (lines 237-241, read directly, unmodified by this feature). This means the new Enter branch cannot reach `SelectRow` with an invalid target — the pre-existing fail-fast guard already covers this new caller. No new guard was needed and none was skipped.
+
+## Finding 3 (#693) — Router test assertion fix — `FolderBreadcrumbBridgeRouterTests.cs`
+
+```
+- await ArrowAsync(router, "left");
+- await ArrowAsync(router, "left");
++ var firstPress = await ArrowAsync(router, "left");
++ var secondPress = await ArrowAsync(router, "left");
+  ...
++ firstPress.Should().ContainSingle();
++ BreadcrumbBridgeSerializer.Parse(firstPress[0]).Should().BeOfType<RenderMessage>();
++ secondPress.Should().ContainSingle();
++ BreadcrumbBridgeSerializer.Parse(secondPress[0]).Should().BeOfType<RenderMessage>();
+```
+
+- Correctly captures both previously-discarded results and asserts each parses to a `RenderMessage`, matching the file's own sibling pattern at `Route_ThemeChange_EchoesThemeAndReRenders` (line 408: `BreadcrumbBridgeSerializer.Parse(outputs[1]).Should().BeOfType<RenderMessage>();`), so the assertion idiom is consistent with the rest of the file rather than introducing a new style.
+- `ArrowAsync`, `PopulatedRouterAsync`, and all three provider-mock factories (`ProviderMock`, `StemProviderMock`, `ParentSubfolderProviderMock`) are unmodified — confirmed by diff (only the body of `Route_LeftArrow_NothingToCollapse_ReportsUnhandledLeft` changed).
+- The fix is verification-only: it does not alter the router's production behavior, and the isolated pass of the modified test plus the pass of the sibling `ArrowAsync_QfcLeftOnMultiSegmentRow_RoutesParentSelectTransition` test (both recorded in QA-gate evidence) independently corroborate that the two newly-asserted `RenderMessage` outcomes are consistent with the existing #440 ancestor-walk contract, not a coincidental pass.
+
+## New Test — `BreadcrumbHtmlRendererTests.cs`
+
+```
+[TestMethod]
+public void Issue737BridgeJsPostsRowSelectedOnEnterAndScrollsSelectedRowIntoView()
+{
+    string bridgeJs = BreadcrumbDocumentAssets.BridgeJs;
+    bridgeJs.Should().Contain("e.key === 'Enter'");
+    bridgeJs.Should().Contain("post({ type: 'rowSelected', rowId: id });");
+    bridgeJs.Should().Contain("scrollIntoView({ block: 'nearest' })");
+}
+```
+
+- Follows the file's established `Issue439ActiveAncestorChildrenAndEmbeddedBridgeUseTypedStoppedActivation` precedent exactly (string-containment against the public `BridgeJs` constant), placed immediately after it in the file.
+- XML-doc `<summary>` explicitly documents the JS-execution-harness limitation (string-shape verification, not runtime-behavior verification), satisfying the spec's Test Strategy requirement to record this limitation in the test's own comment.
+- The `post({ type: 'rowSelected', rowId: id });` assertion string is a substring shared between this new test's target text and the file's Enter-branch source; it is distinct from the pre-existing click handler's `post({ type: 'rowSelected', rowId: rowId });` (different variable name, `id` vs `rowId`), so this assertion cannot pass against pre-existing code accidentally — it is a meaningful, discriminating check, not a tautology.
+
+## Design Principles (General Code Change Policy §1)
+
+- **Simplicity first**: PASS. Both JS additions are minimal, linear insertions matching the existing file's authoring style (string-literal concatenation).
+- **Reusability**: PASS with the one minor duplication note above (spec-mandated, not a new pattern).
+- **Extensibility**: N/A — no public API surface changed.
+- **Separation of concerns**: PASS. The JS content stays entirely within the existing `BridgeJs` constant; no C#-side wiring, message type, or router logic was touched, correctly respecting the Efc/Qfc pipeline boundary this spec explicitly documents and enforces via its Write Set.
+
+## Naming, Comments, Documentation
+
+- New test method name (`Issue737BridgeJsPostsRowSelectedOnEnterAndScrollsSelectedRowIntoView`) is descriptive and follows the file's existing `IssueNNN...` naming convention.
+- New local variables in the router test (`firstPress`, `secondPress`) are clearly named and communicate their Arrange-phase role better than the previous discarded, unnamed `await` calls.
+- No new production comments were added explaining "why" for the JS changes; given the change is a direct, spec-traceable, one-purpose insertion with an XML-doc comment on the class already describing the bridge's message contract, this is acceptable and not a gap.
+
+## Blocking Findings
+
+None.
+
+## Non-Blocking Observations (for future awareness, not remediation-required)
+
+1. The `.rowwrap.selected` lookup pattern is duplicated across the Enter and arrow-key `keydown` branches (spec-mandated mirror of existing style; a future JS-content refactor could extract a shared `getSelectedRowId()` local function).
+2. The new post-render scroll-into-view call is not textually gated to the `render`/`subfolderResult` branches specifically, though it is currently behaviorally equivalent since those are the listener's only two recognized message types.
+
+Neither observation blocks merge or requires remediation-inputs.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-analyzer-rebuild.2026-09-02T00-00.md

@@ -0,0 +1,13 @@
+Timestamp: 2026-09-03T01-20
+
+Command: pwsh -File scripts\vscode\Invoke-VSBuild.ps1 -Target Rebuild -EnableNETAnalyzers -EnforceCodeStyleInBuild
+
+EXIT_CODE: 0
+
+Output Summary: "Build succeeded." followed by "5 Warning(s)" and "0 Error(s)". All 5
+warnings are the identical pre-existing `System.Reactive.PackagesConfigCheck.targets`
+message ("The project contains a packages.config file, which is not supported by
+System.Reactive v7.0 or later...") repeated once per project referencing
+System.Reactive 7.0.0 (UtilitiesCS, ToDoModel, QuickFiler, TaskMaster,
+UtilitiesCS.Test). This is the baseline warning set for the Phase 5 restart rule
+comparison. No error diagnostics.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-coverage-fullrepo.2026-09-02T00-00.md

@@ -0,0 +1,13 @@
+Timestamp: 2026-09-03T01-31
+
+Command: pwsh -File scripts\vscode\Invoke-MSTestWithCoverage.ps1 -SearchRoot . -CoverageOutput coverage\breadcrumb-737-baseline.cobertura.xml
+
+EXIT_CODE: 0
+
+Output Summary: "Test Run Successful." "Total tests: 6955" "Passed: 6955" (48.4842
+Seconds). Coverage artifact written to
+coverage\breadcrumb-737-baseline.cobertura.xml and post-processed for Koverage
+compatibility. Read from the emitted Cobertura XML's `/coverage` element:
+- line-rate = 0.853836 (85.3836%)
+- lines-covered = 55139
+- lines-valid = 64578

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-csharpier-check.2026-09-02T00-00.md

@@ -0,0 +1,10 @@
+Timestamp: 2026-09-03T01-17
+
+Command: dotnet tool run csharpier check .
+
+EXIT_CODE: 0
+
+Output Summary: "Checked 1571 files in 5620ms." No unformatted-file list was printed
+(CSharpier prints a per-file "Was not formatted." line only for files that fail the
+check; none appeared). Baseline comparison set for the Phase 5 restart rule: EMPTY (zero
+files reported as unformatted at baseline).

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-nuget-restore.2026-09-02T00-00.md

@@ -0,0 +1,15 @@
+Timestamp: 2026-09-03T01-15
+
+Command: pwsh -File scripts\vscode\Invoke-Restore.ps1
+
+EXIT_CODE: 0
+
+Output Summary: MSBuild resolved via vswhere to
+`C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Current\Bin\MSBuild.exe`
+(MSBuild version 18.9.1). The `packages.config` restore of `TaskMaster.sln` ran the
+`Restore` target across all first-party and vendored projects, adding/restoring every
+NuGet package referenced by the solution (FluentAssertions, Moq, MSTest.*, Meziantou,
+SonarAnalyzer, Roslynator, AsyncFixer, Microsoft.CodeAnalysis.BannedApiAnalyzers, etc.)
+into the worktree-local `packages\` folder. Terminal summary line: "Build succeeded."
+followed by "0 Warning(s)" and "0 Error(s)". The invoking pwsh process's own exit code
+was 0.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-nullable-rebuild.2026-09-02T00-00.md

@@ -0,0 +1,11 @@
+Timestamp: 2026-09-03T01-24
+
+Command: pwsh -File scripts\vscode\Invoke-VSBuild.ps1 -Target Rebuild -TreatWarningsAsErrors
+
+EXIT_CODE: 0
+
+Output Summary: "Build succeeded." followed by "5 Warning(s)" and "0 Error(s)". Same 5
+pre-existing `System.Reactive.PackagesConfigCheck.targets` warnings observed in the
+P0-T11 analyzer-rebuild baseline (one per project referencing System.Reactive 7.0.0);
+no nullable (CS86xx) diagnostics were promoted to errors, confirming a clean nullable
+baseline with `/p:TreatWarningsAsErrors=true`.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-sdk-bootstrap.2026-09-02T00-00.md

@@ -0,0 +1,13 @@
+Timestamp: 2026-09-03T01-12
+
+Command: pwsh -File scripts\vscode\Install-RepoDotNetSdk.ps1
+
+EXIT_CODE: 0
+
+Output Summary: "Downloading .NET SDK 8.0.205 from
+https://builds.dotnet.microsoft.com/dotnet/Sdk/8.0.205/dotnet-sdk-8.0.205-win-x64.zip..."
+followed by "Installed repo-local .NET SDK 8.0.205 to
+C:\Users\DanMoisan\repos\TaskMaster\.claude\worktrees\agent-aafe0f7ad44246375\.dotnet-sdk."
+The pwsh process completed with no thrown error. Verified the resolved SDK version
+directly by invoking the repo-local `.dotnet-sdk\dotnet.exe --version`, which printed
+`8.0.205`, confirming the resolved SDK version line the script installs.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-tool-restore.2026-09-02T00-00.md

@@ -0,0 +1,10 @@
+Timestamp: 2026-09-03T01-14
+
+Command: dotnet tool restore
+
+EXIT_CODE: 0
+
+Output Summary: "Tool 'csharpier' (version '1.2.6') was restored. Available commands:
+csharpier" followed by "Restore was successful." Confirms the manifest-pinned CSharpier
+1.2.6 tool (from dotnet-tools.json) is restored and available for `dotnet tool run
+csharpier` invocations in this worktree.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/baseline-vstest-scoped.2026-09-02T00-00.md

@@ -0,0 +1,17 @@
+Timestamp: 2026-09-03T01-27
+
+Command: vstest.console.exe (vswhere-resolved from
+"C:\Program Files\Microsoft Visual Studio\18\Community\Common7\IDE\Extensions\TestPlatform\vstest.console.exe")
+UtilitiesCS.Test\bin\Debug\UtilitiesCS.Test.dll /Settings:scripts\vscode\TaskMaster.cli.runsettings
+/InIsolation /TestCaseFilter:"FullyQualifiedName~FolderBreadcrumbBridgeRouterTests|FullyQualifiedName~BreadcrumbHtmlRendererTests"
+"/Logger:trx;LogFileName=baseline-scoped.trx"
+/ResultsDirectory:docs\features\active\2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737\evidence\baseline\testresults
+
+EXIT_CODE: 0
+
+Output Summary: "Test Run Successful." followed by "Total tests: 40" and "Passed: 40".
+This matches the plan's stated expected baseline of Total: 40 (14 in
+FolderBreadcrumbBridgeRouterTests.cs, 12 in the sibling partial
+FolderBreadcrumbBridgeRouterInFlightTests.cs, 14 in BreadcrumbHtmlRendererTests.cs),
+Failed: 0. TRX written to
+evidence\baseline\testresults\baseline-scoped.trx.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/baseline/phase0-instructions-read.2026-09-02T00-00.md

@@ -0,0 +1,16 @@
+Timestamp: 2026-09-03T01-09
+
+Policy Order:
+1. CLAUDE.md
+2. .claude/rules/general-code-change.md
+3. .claude/rules/general-unit-test.md
+4. .claude/rules/csharp.md
+5. .claude/rules/quality-tiers.md
+
+Files read (in order, in full, from the item worktree at
+C:/Users/DanMoisan/repos/TaskMaster/.claude/worktrees/agent-aafe0f7ad44246375):
+1. CLAUDE.md
+2. .claude/rules/general-code-change.md
+3. .claude/rules/general-unit-test.md
+4. .claude/rules/csharp.md
+5. .claude/rules/quality-tiers.md

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/qa-gates/qa-analyzer-rebuild.2026-09-02T00-00.md

@@ -0,0 +1,9 @@
+Timestamp: 2026-09-03T01-56
+
+Command: pwsh -File scripts\vscode\Invoke-VSBuild.ps1 -Target Rebuild -EnableNETAnalyzers -EnforceCodeStyleInBuild
+
+EXIT_CODE: 0
+
+Output Summary: "Build succeeded." followed by "5 Warning(s)" and "0 Error(s)". Matches
+the P0-T11 baseline exactly (same 5 pre-existing System.Reactive packages.config
+warnings); no new analyzer diagnostics introduced by the Write Set changes.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/qa-gates/qa-build-phase3.2026-09-02T00-00.md

@@ -0,0 +1,10 @@
+Timestamp: 2026-09-03T01-40
+
+Command: pwsh -File scripts\vscode\Invoke-VSBuild.ps1 -Target Rebuild
+
+EXIT_CODE: 0
+
+Output Summary: "Build succeeded." followed by "5 Warning(s)" and "0 Error(s)". Warning
+count matches the P0-T11/P0-T12 baseline exactly (the pre-existing System.Reactive
+packages.config warnings); the Phase 1/2 JS edits and the new Phase 3 test method
+compiled cleanly with no new diagnostics.

docs/features/active/2026-09-02-breadcrumb-bridge-keyboard-navigation-defects-737/evidence/qa-gates/qa-build-phase4.2026-09-02T00-00.md

@@ -0,0 +1,9 @@
+Timestamp: 2026-09-03T01-45
+
+Command: pwsh -File scripts\vscode\Invoke-VSBuild.ps1 -Target Rebuild
+
+EXIT_CODE: 0
+
+Output Summary: "Build succeeded." followed by "5 Warning(s)" and "0 Error(s)". Matches
+baseline exactly; the Finding 3 test-assertion edit (including the `RenderMessage` type
+reference used in the new assertions) compiled cleanly with no new diagnostics.