fix(quickfiler): validate breadcrumb segment index and restore Left/Right tree navigation - #626
Merged
drmoisan merged 12 commits intoAug 26, 2026
Conversation
…capture baselines (#498 phases 0-1) Phase 0 captures the toolchain baseline for the breadcrumb router work and Phase 1 performs the mandatory decision-D8 partial-class split. Baselines (all measured in this worktree): csharpier check exit 0 over 1520 files; analyzer Rebuild exit 0 with 0 errors; nullable Rebuild exit 0 with 0 errors; full suite 6482 of 6482 passing with an empty failure set; repository line rate 84.78 percent. The analyzer and nullable baselines initially read exit 1 with 10 CS0006 errors. That was a provisioning gap in a fresh worktree, not a repository baseline: analyzer package version skew leaves 16 project files naming superseded Analyzer Include paths. Two gitignored package directories were provisioned to reach parity with CI. No tracked file changed. See evidence/other/orchestrator-cross-cutting-findings for the full analysis and the recommended upstream repair. Phase 1 relocates twelve private members into BreadcrumbBridgeRouter.Selection.cs, taking the router from 596 to 410 lines. Pure mechanical relocation: the same 40 router tests pass before and after, verified by diffing test names between both TRX files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
Phase 2 of the breadcrumb-router-navigation-defects plan. The SegmentDoubleClick arm of BreadcrumbBridgeRouter.ProcessInboundAsync dereferenced message.SegmentIndex with the null-forgiving operator and passed it straight to BreadcrumbRow.CollapseAfter, which throws ArgumentOutOfRangeException for an index outside the row's segment range. On the async void OnHostMessageReceived seam that exception was unreachable by the single catch (BreadcrumbMessageException), so it escaped the boundary onto a thread-pool thread. The arm now reads the index into a local, rejects a missing or out-of-range value, logs the rejection at Error through the existing log4net field, and returns without a transition and without a render post. BreadcrumbRow is unmodified and its documented throw contract still holds for direct callers. The host-message catch was not widened. Tests: BreadcrumbBridgeRouterQueueTests was made partial (decision D8) and a Part2 sibling added, carrying two direct-path regression tests recorded RED before the fix and GREEN after, plus four host-event, valid-index and banner-short-circuit controls. Gates: csharpier check EXIT 0; analyzer Rebuild EXIT 0 (5 pre-existing System.Reactive packages.config warnings, 0 errors); nullable Rebuild EXIT 0; scoped runs p2-t3 2/0/2 RED, p2-t5 2/2/0, p2-t6 4/4/0. Acceptance criteria satisfied: AC-1, AC-2, AC-3, AC-25. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
Phase 3 of the breadcrumb-router-navigation-defects plan. BindRowsAsync reset _selectedRowId but left SelectedFolderPath holding the folder chosen against the previous row set, and raised no notification. A caller reading the property after a re-bind therefore acted on a folder the user could no longer see. The internal BindRowsAsync overload now clears SelectedFolderPath alongside the row-id reset and raises SelectedFolderPathChanged with null, guarded so the notification fires only when the previous value was non-null. A re-bind with no prior selection stays silent. No auto-selection was introduced: SelectFirstRow is still called only from EfcFormController. The two existing SelectedFolderPath write sites, SelectRow and SelectHierarchyPath, are unchanged. Tests: six methods added to BreadcrumbBridgeRouterQueueTests.Part2.cs — two regression tests recorded RED before the fix and GREEN after, two write-site preservation guards, and two no-side-effect tests for the conditional notification and the absence of auto-selection. Gates: csharpier check EXIT 0; analyzer Rebuild EXIT 0 (5 pre-existing System.Reactive packages.config warnings, 0 errors); nullable Rebuild EXIT 0; scoped runs p3-t2 2/0/2 RED, p3-t4 2/2/0, p3-t5 2/2/0, p3-t6 2/2/0, p3-t7 2/2/0; full BreadcrumbBridgeRouter class sweep 52/52. Acceptance criteria satisfied: AC-4, AC-5, AC-6, AC-26. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
…ch (#440) Phase 4 of the #498 plan: the Qfc ancestor-chain prerequisite for #440. OutlookFolderHierarchyProvider.ResolveLeafKeyAsync keeps its exact OrdinalIgnoreCase first pass unchanged and returns early on a hit, so the change is a strict no-op for the Efc surface. When the exact pass misses, a second pass accepts a node whose FolderPath ends with a directory separator followed by the requested path, and only when exactly one node qualifies. Zero or multiple candidates log at Error and return null, preserving today's single-segment fallback rendering. Tests (RED first, per the bugfix workflow): the suffix-match regression test, the Efc full-path no-op boundary, the ambiguous-decoy boundary, and the Qfc multi-segment lineage assertion against a MockBehavior.Strict provider. Also records the decision-D7 read-only verification, which selects rung 1. Satisfies AC-7, AC-8, AC-9, AC-11 and AC-27. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
) Phase 5 of the #498 plan: the decision-D7 ladder, rung 1. With the Phase 4 resolution in place a Qfc suggestion row now carries a resolved chain, so BreadcrumbSelectionMap.GetSelectedFolder switched from the presented archive-relative stem to the store-qualified leaf path, silently changing what the row files into. BreadcrumbStateRow gains an internal constructor overload taking the presented filing target, backed by a private WithFilingTarget that replaces the leaf segment's FolderPath while keeping its Key, DisplayName and HasChildren. SetSuggestionsAsync constructs resolved suggestion rows through it. Rendering reads DisplayName and navigation reads Key, so the substitution is confined to the filing value and BreadcrumbSelectionMap.cs is not written. Rung 1 was selected by the P4-T1 read-only verification and is recorded in the spec's RISK-1 entry; rungs 2 and 3 are recorded NOT APPLICABLE. Satisfies AC-14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
Left now attempts BreadcrumbRow.ActivateSegment on the segment one step toward the root before falling through to the pre-existing row.LeftArrow() collapse behavior. Right attempts a tree transition first: it clears any collapse through row.ReExpand() as part of the transition, expands the active non-leaf segment through the landed ExpandLeafAsync (a single GetImmediateSubfoldersAsync call keyed on ActiveSegmentKey, with no ResolveLeafKeyAsync), and once expanded descends by activating child index 0 (decision D9) through row.GetActiveChild(0) and SelectHierarchyPath. Arrow-key and leaf-expansion members were relocated into the new partial sibling BreadcrumbBridgeRouter.Arrows.cs, and the router test class into BreadcrumbBridgeRouterTests.Selection.cs, so that no file exceeds the 500-line limit. Both relocations are mechanical and change no behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
BreadcrumbStateRow gains the #440 selected-node state — ActiveSegmentIndex, ActiveSegment, ActiveSegmentHasSubfolders, ActivateSegment, GetActiveChild and TryExpandActiveSegment. BreadcrumbStateModel.LeftArrow now selects the parent of the leaf-anchored node before the pre-existing collapse path, and RightArrow expands the selected node (clearing any collapse as part of the transition) and then descends into child index 0, falling through to the pre-existing behavior whenever no transition applies. BreadcrumbStateRow moved into the new sibling file BreadcrumbStateModel.Row.cs so that neither file exceeds the 500-line limit. Route_LeftArrow_NothingToCollapse_ReportsUnhandledLeft now presses Left twice: its purpose is unchanged, but under the #440 contract the first Left consumes the one available parent-select transition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
…440) FolderBreadcrumbBridgeRouter now keys its subfolder query on the row's ACTIVE node rather than unconditionally on the leaf, so the expansion that follows a parent-select queries the selected node. The UnhandledArrowMessage emission in ArrowAsync is unchanged and remains the fall-through. FolderBreadcrumb.html onArrow no longer pre-filters a suggestion row by its view state, which previously suppressed both #440 transitions before they could reach the C# router. The arrowKey and unhandledArrow message shapes are unchanged. Adds the decision-D1 handling-order tests and the decision-D2 boundary preservation tests on both surfaces, and records the decision-D9 descent mechanism in spec.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
) Phase 7 of the breadcrumb-router-navigation-defects plan. Records the residual 500-line sweep (no split needed; max owned file 495 lines), the ownership diff against the Phase 0 baseline commit (18 in-scope paths, all OWNED, zero forbidden paths), and four scoped verification runs: FolderBreadcrumbAssetContractTests 15/15, BreadcrumbStateModelSelectorTests 9/9, BreadcrumbStateModelTests 32/32 and BreadcrumbBridgeRouterIssue439Tests 10/10, all with zero failures. Also records the AC-9 supersession record check. Satisfies AC-19, AC-20, AC-21, AC-22, AC-30 and AC-31. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
…498) Phase 8 of the breadcrumb-router-navigation-defects plan: the mandatory C# toolchain loop, the coverage delta and the acceptance-criteria walk. The first pass measured BreadcrumbStateModel.Row.cs changed-line coverage at 89.56%, under the plan's 90% floor. Diagnosis: 13 of its 19 uncovered lines were relocated lines already uncovered at baseline, but 6 were newly authored guard branches with no test - WithFilingTarget's null/empty chain early return, GetActiveChild's invalid-request null, and TryExpandActiveSegment's no-affordance false. Three deterministic tests were added to the owned BreadcrumbStateModelTests.cs to cover them, which raised the file to 92.86% and forced the toolchain loop to restart. Final pass (3) is clean at every step: csharpier format rewrote 0 files (SHA-256 verified), csharpier check exit 0 over 1525 files, analyzer Rebuild 0 errors, nullable Rebuild 0 errors, full suite 6514/6514 passed 0 failed. Coverage: repository line rate 84.78% -> 84.83%; changed-line coverage 94.90% over newly authored code; the relocated Selection.cs partial measures 97.71% against a 97.71% baseline, i.e. zero regression. All 31 acceptance criteria are now checked in spec.md: 28 satisfied by this feature, 3 retired as inherited from PR #605, 0 unmet. Satisfies AC-29. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
Remove the operating-system account name, machine name, and absolute worktree paths from the 32 vstest TRX artifacts, one prose evidence file, and the research header in this feature folder. The redaction runs after the final plan task, so every gate in Phases 0 through 8 was executed and recorded against unmodified tool output. Per file, the multiset of test outcomes and the set of test names are unchanged from the committed versions, and all 32 TRX remain well-formed XML, so no gate conclusion loses the artifact that supports it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mic58ikwEhpXsTnhz9FShE
…ration' into bug/breadcrumb-router-navigation-defects-498
drmoisan
merged commit Aug 26, 2026
8c8f769
into
epic/quickfiler-bug-family-integration
5 checks passed
drmoisan
added a commit
that referenced
this pull request
Aug 26, 2026
Projects the epic checkpoint after the fan-in-only run: 484 (PR #619), 446 (PR #625) and 498 (PR #626) are merged into the integration branch, whose tip is now 8c8f769. Records that 468 is halted rather than merged. Its stale-base merge is pushed and its CI run is green, but its atomic plan is 120 of 180 tasks complete (P13 stops at T3; P14, P15 and P16 never ran) and 14 of 29 acceptance criteria in spec.md are unchecked, so no pull request was opened for it. Also carries forward the child-PR CI trigger gap, the missing feature-review artifacts, and the worktree-removal gate defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drmoisan
added a commit
that referenced
this pull request
Aug 26, 2026
Projects the epic checkpoint after feature 468 merged as PR #636. Four of twelve features are now on the integration branch: 484 (#619), 446 (#625), 498 (#626) and 468 (#636), whose merge is the current tip 808bf46. Records that 468 landed on a second pass. It was halted earlier in this session at 120 of 180 plan tasks with 14 of 29 acceptance criteria unchecked and no feature review; it was re-delegated to resume at P13-T4 and is now 180 of 180 tasks and 28 of 29 criteria with three audit artifacts carrying zero blocking findings. AC-28 remains unchecked by design, because an integration-branch merge cannot close the seven referenced issues. Also records the repository-wide line-coverage shortfall against the rules-file floor and the absence of feature-review artifacts for 498. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(quickfiler): validate breadcrumb segment index and restore Left/Right tree navigation
Summary
SegmentIndexvalues in theSegmentDoubleClickarm ofBreadcrumbBridgeRouter.ProcessInboundAsync, closing a host-crash path where an unvalidated index reachedBreadcrumbRow.CollapseAfterand threw out of anasync voidhandler that caught onlyBreadcrumbMessageException.row.LeftArrow()and Right toReExpand/ExpandLeafAsyncinstead of the tree transitions.SelectedFolderPathon re-bind, soBindRowsAsyncno longer leaves a selection pointing at a folder from the previous binding.BreadcrumbBridgeRouterandBreadcrumbStateModelinto partial classes (.Arrows,.Selection,.Row) to stay within the 500-line file cap; this is the largest mechanical portion of the diff.Why
Three open defects in the breadcrumb navigation stack share one component surface, so they are fixed together rather than in three serialized passes over the same files.
#498is a crash, not a cosmetic defect:ProcessInboundAsynccalledrow.CollapseAfter(message.SegmentIndex!.Value)with no range check (BreadcrumbBridgeRouter.cs:241-247),BreadcrumbRow.CollapseAfterthrows on an out-of-range index (BreadcrumbRow.cs:207-214), andOnHostMessageReceivedwasasync voidcatching onlyBreadcrumbMessageException(:266-277) — so the throw escaped to the host.#499left_selectedRowId = nullinBindRowsAsync(:136) while never clearingSelectedFolderPath(:59), leaving the two fields disagreeing after a re-bind.#440was unfixed on both surfaces:HandleArrowKeyAsync(:304-339),BreadcrumbStateModel.RightArrow/LeftArrow(:424-437,:443-455),FolderBreadcrumbBridgeRouter.ArrowAsync(:378-406),KeyboardHandler.BreadcrumbArrowFallThrough(:288-315), andFolderBreadcrumb.htmlwere all untouched by previously landed work.#439(ancestor lineage never resolving) is inherited as already fixed onmainby PR #605 and is not re-implemented or closed here.What Changed
Core fixes
QuickFiler/Controllers/BreadcrumbBridgeRouter.cs— range-checksSegmentIndexbeforeCollapseAfter; clearsSelectedFolderPathon re-bind; reduced from a single 354-line-change file by the partial-class split below.QuickFiler/Controllers/BreadcrumbBridgeRouter.Arrows.cs(new, 211 lines) — Left/Right arrow handling routed to tree transitions.QuickFiler/Controllers/BreadcrumbBridgeRouter.Selection.cs(new, 204 lines) — selection and re-bind state.UtilitiesCS/OutlookObjects/Folder/BreadcrumbStateModel.cs+BreadcrumbStateModel.Row.cs(new, 384 lines) — state-model tree navigation.UtilitiesCS/OutlookObjects/Folder/FolderBreadcrumbBridgeRouter.cs,OutlookFolderHierarchyProvider.cs— Efc-side arrow navigation and archive-relative stem resolution by unique suffix match.Tests
Seven files, 1,332 insertions:
BreadcrumbBridgeRouterQueueTests(.Part2),BreadcrumbBridgeRouterTests(.Selection),BreadcrumbStateModelTests,FolderBreadcrumbBridgeRouterTests,OutlookFolderHierarchyProviderTests.Merge commit
This branch was 30 commits behind
epic/quickfiler-bug-family-integration. Its pre-merge diff against the base contained pure deletions of feature 484's entire evidence tree —code-review.2026-08-26T10-22.md, all tenevidence/baseline/*artifacts,policy-audit.2026-08-26T10-22.md, and 80 lines ofepic-status.md— which would have merged silently, with no conflict, and erased what PR #619 had just contributed.origin/epic/quickfiler-bug-family-integrationwas merged in at4c54d418to correct this. The diff against the base is now verified to contain zero pure-deletion files.Architecture / How It Fits Together
The breadcrumb stack spans two hosts.
BreadcrumbStateModel(UtilitiesCS) owns the row/segment tree and its transitions. Two routers adapt it:FolderBreadcrumbBridgeRouterfor the Efc folder surface andBreadcrumbBridgeRouterfor the Qfc surface. Inbound host messages enter throughProcessInboundAsync; keyboard input enters throughHandleArrowKeyAsyncandKeyboardHandler.BreadcrumbArrowFallThrough.The fixes place validation at the router boundary (where untrusted
SegmentIndexvalues arrive) and the navigation semantics in the state model (where the tree is actually known), rather than duplicating range logic in both routers.Verification
Completed
Recorded under
docs/features/active/breadcrumb-router-navigation-defects-498/evidence/qa-gates/; final pass is pass 3, a single clean pass with no degradation on any step:dotnet tool run csharpier formatEXIT_CODE: 0; 0 files rewritten (SHA-256 before/after)dotnet tool run csharpier check .EXIT_CODE: 0; 1525 files checked, none unformattedmsbuild /t:Rebuild /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=trueEXIT_CODE: 0; 0 errors, 5 warningsmsbuild /t:Rebuild /p:TreatWarningsAsErrors=trueEXIT_CODE: 0; 0 errors, 5 warningsvstestwith Cobertura coverageEXIT_CODE: 0; 6514/6514 passed, 0 failed; line rate 84.83%Coverage delta (
p8-t7): repository line coverage 84.78% → 84.83%; changed-line coverage over newly authored code 94.90%; every per-file changed-line figure at or above the 90.00% floor.All 31 acceptance criteria in
spec.mdare checked; none remain open.Recommended
Backward Compatibility / Migration Notes
BreadcrumbBridgeRouterandBreadcrumbStateModelwere split into partial classes; the type names, namespaces, and member signatures are unchanged, so callers are unaffected.SegmentIndexvalues that previously crashed the host are now rejected. Any caller that relied on the exception escaping will see a rejection instead — no in-repo caller does.Risks and Mitigations
.Arrows,.Selection, and.Rowfiles. Mitigation: review the new files against the originals first, then the behavioral commits; see the Review Guide.unhandledArrowand closes the drop-down; Right on a childless node: Efc silent no-op). This is specified, not accidental. Mitigation: the divergence is documented inspec.mdand pinned by tests.4c54d418and the eleven feature commits; no schema, migration, or persisted-state change is involved.Review Guide
BreadcrumbBridgeRouter.cs— theSegmentIndexrange check and theSelectedFolderPathclear (the two smallest, highest-value changes).BreadcrumbBridgeRouter.Arrows.csandBreadcrumbStateModel.Row.cs— the new navigation semantics.OutlookFolderHierarchyProvider.cs— archive-relative stem resolution by unique suffix match.BreadcrumbBridgeRouter.cs/BreadcrumbStateModel.cs, and the evidence artifacts underdocs/features/active/breadcrumb-router-navigation-defects-498/evidence/(large, mechanical, includes.trxoutput).Follow-ups
#439remains open upstream as inherited-fixed via PR Restore EfcViewer folder lineage and segment navigation #605; confirm and close separately if that fix is judged complete.spec.mdpending a maintainer decision.GitHub Auto-close
Source:
spec.md:4— "Also closes: #440, #499" — with#498as the feature folder's own issue. All three verifiedOPENviagh issue viewat authoring time.#439is deliberately not closed here: it is inherited as already fixed onmainby PR #605.