refactor(workstation): migrate submodule/remote/issue/PR-triage cursors to id-based selection#1645
Open
gfargo-horizon-agent[bot] wants to merge 2 commits into
Conversation
…election (#1452 phase 3) Extends the #1452 dual-write + selector-flip migration to the submodules (id: path) and remotes (id: name) views, replicating the pattern proven for worktrees in 4b641a9. Consolidates the scattered inline filteredSubmoduleList[index]/filteredRemoteList[index] resolution across useWorkflowAction.ts and useYankActions.ts onto new getSelectedSubmodule/ getSelectedRemote selectors.
…lection (#1452 phase 3 continued) Completes the dual-write + selector-flip migration for the issues and pull-request-triage views, following the just-shipped submodule/remote pattern (#1452). Action/yank target resolution now keys off the stringified issue/PR number instead of the raw selected*Index, so a background list refetch that reorders rows can't silently retarget an in-flight triage action. - Add getSelectedIssue/getSelectedIssueId and getSelectedPullRequestTriage/getSelectedPullRequestTriageId to selection.ts, matching the id-first/index-fallback/unfiltered-fallback shape of getSelectedSubmodule/getSelectedRemote. - Add selectedIssueId/selectedPullRequestTriageId mirrors to LogInkState; moveIssue/movePullRequestTriage actions gain id?, and every reducer case that resets/rectifies the index also clears (or precisely sets, for navigateOpenDiffForPullRequest) the mirror. - Flip all filteredIssueList/filteredPullRequestTriageList inline resolution in useWorkflowAction.ts and useYankActions.ts onto the new selectors; drop the now-dead filtered*List deps and type aliases. - Thread issueListIds/pullRequestTriageListIds through useInputHandler.ts and inkInput.ts so moveIssue/movePullRequestTriage dispatch sites resolve the post-move id via resolveMoveTargetId.
gfargo-horizon-agent
Bot
changed the title
![gfargo-horizon-agent[bot]](https://avatars.githubusercontent.com/in/4083186?s=60&v=4){kind=small}
Contributor
Author
There was a problem hiding this comment.
🔎 Agent review (sonnet→opus) — LGTM
REVIEW: LGTM
RESOLVES: full
PR #1645 cleanly completes the #1452 id-based selection flip for submodules, remotes, issues, and PR-triage — the new selectors replicate buildFilteredLists's filter-field arrays verbatim, the reducer writes/clears every id mirror at the required sites, and resolveMoveTargetId's clamping matches clampIndex so id and index never diverge. tsc, lint (only a pre-existing exhaustive-deps warning that also exists on main), and all 762 touched-suite tests pass; no correctness issues found.
- 🧹 PR bundles submodule/remote with issue/PR-triage — The implementation plan scoped this PR to issues/PR-triage only, assuming submodule/remote id-based selection had already shipped separately — but no such PR is on main, so this one delivers both migrations together (two commits). It stays entirely within OSS-769's overall scope and touches only the relevant runtime files with no unrelated changes, so this is a scope-size observation rather than a defect.
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
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.
What
Extends the id-based selection migration (#1452, dual-write + selector-flip) to the submodules (id = submodule path), remotes (id = remote name), issues (id = stringified issue number), and pull-request-triage (id = stringified PR number) views, replicating the pattern proven for worktrees in
4b641a9. Consolidates the scattered inlinefiltered*List[index]resolution inuseWorkflowAction.tsanduseYankActions.tsonto single selectors per view.Why
Closes #1569
Plane: OSS-769
File-history and the history/diff/status-file trio are explicitly deferred to follow-up PRs per the item's suggested order (bigger surface, not a drop-in repeat of this pattern).
How
inkViewModel.ts:selectedSubmoduleId?/selectedRemoteId?/selectedIssueId?/selectedPullRequestTriageId?state fields;moveSubmodule/moveRemote/moveIssue/movePullRequestTriageactions dual-write the id alongside the index; all cleared on repo-frame push/pop and filter-preset cycling;navigateOpenDiffForPullRequestsets the PR id precisely (the PR number IS the target's id).selection.ts: newgetSelectedSubmodule/getSelectedSubmoduleId,getSelectedRemote/getSelectedRemoteId,getSelectedIssue/getSelectedIssueId, andgetSelectedPullRequestTriage/getSelectedPullRequestTriageIdselectors — id-first, index-fallback, with an unfiltered-list fallback tier (same shape asgetSelectedWorktree). The issue/PR filter fields mirrorbuildFilteredLists.ts's promoted-filter arrays exactly.inkInput.ts/useInputHandler.ts: threadsubmoduleListIds/remoteListIds/issueListIds/pullRequestTriageListIdsthrough the input context so the move dispatch sites can resolve the post-move target id viaresolveMoveTargetId.useWorkflowAction.ts: the submodule-init/update/sync, remote-set-url/remove/prune, and alltriage-issue-*/triage-pr-*call sites now resolve through the new selectors instead of inlinefiltered*List[Math.min(index, …)]; removed the now-deadfiltered*Listdeps and type aliases;resolvePendingItemAction'striage-pr-checkoutspinner target also routes through the selector.useYankActions.ts: submodule, remote, issue, and PR-triage yank arms resolve through the same selectors; removed the now-deadfilteredIssueList/filteredPullRequestTriageListdeps.Testing
selection.test.ts,inkViewModel.test.ts,inkInput.test.ts— id-first/index-fallback/unfiltered-fallback cases, reducer dual-write/push-pop-clear/filter-cycle-clear cases, and move-dispatch id resolution, mirroring the worktree/submodule/remote additions)🤖 Generated by the harbor agent loop. Reviewed by a human before merge.
Closes #1569