Skip to content

🐛 Activity Monitor: secondary issue of a shared PR rides the PR's lane - #17325

Merged
clubanderson merged 1 commit into
v5from
fix/shared-pr-secondary-row-lane
Nov 9, 2026
Merged

clubanderson merged 1 commit into
v5from
fix/shared-pr-secondary-row-lane

Conversation

@clubanderson

Copy link
Copy Markdown
Member

Fixes #17324

A PR that closes two open issues is drawn once, under the lower issue (drawn_under); the other issue lists it as a compact "↳ #N · under #M" chip. That secondary row dropped the PR from stageTrackerLinkedSorted, so its strip fell back to the issue's own 030/040 stage (the PR in progress lane) while stageTrackerCoveringPR still overlaid the PR's merge-gate word — #17302 read "Soaking · 47s left" in PR in progress while #17292 soaked in the Merge queue.

Fix (static-src/js):

  • stageTrackerLinkedRiding(item, stages): when every covering PR is drawn under another row, the furthest-along open one (same order as stageTrackerLinkedSorted).
  • stageTrackerRowHtml rides that PR (via_pr, stage, merge_gate, caption, …) exactly like a row drawing its one covering PR — but the PR column stays empty and the "under #M" chip still names it.
  • stageTrackerColMap (motion) and stageTrackerLead (list order) use the same rule, so the lane, the move animation and the sort agree.

Tests: TestStageTrackerLinkedDrawnUnderJS now asserts the secondary rides the PR's lane and adds the #17292 / #17302 soak case (same lit chevron on both rows, chip kept, PR column empty); stageTrackerColMap gets a shared-PR column assertion.

A PR closing two issues is drawn once, under the lower issue; the other
issue chips to it. Its row fell back to the issue's own 030/040 stage for
the lane while the merge-gate word still came from the PR, so #17302 read
"Soaking · 47s left" in the PR in progress lane while #17292 soaked in
the Merge queue. stageTrackerLinkedRiding picks the furthest covering PR
drawn elsewhere and the row, column map and sort order ride its stage;
the PR column stays empty and the "under #M" chip still names the PR.

Fixes #17324

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Nov 9, 2026
@clubanderson

Copy link
Copy Markdown
Member Author

/lgtm

@clubanderson

Copy link
Copy Markdown
Member Author

/approve

@kubestellar-prow

Copy link
Copy Markdown
Contributor

@clubanderson: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 9, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clubanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 9, 2026
@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Open PR overlap check

Base branch: v5

Conflicts are checked with every PR merged onto the current tip of v5, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

@hivecommons-hive

hivecommons-hive Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

Another agent tried to open a PR from scanner/fix-17324, but this PR already closes #17324. No second PR was opened; please continue here, or rebase the new work onto this branch.

@clubanderson this PR is held at stage/071-needs-author and the hive will not merge it yet.

Failing checks:

  • overlayfs-exec-guard

Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path.

Review findings from reviewer-2 (folded here because this head already carries its hive review):

Looks correct to me. Read stageTrackerLinkedRiding against the helpers it composes, not just the diff.

  • tracker-row-layout.js:305-310 (new): returns null whenever stageTrackerLinkedSorted is non-empty, so any row that already draws a PR here is untouched — the existing one-PR and several-PR branches in tracker-row-render.js:106-121 keep their old inputs. Only the previously-fallthrough case (all covering PRs drawn under another row) changes.
  • The drawn_under: 0 re-sort trick is sound: stageTrackerLinkedSorted (tracker-row-layout.js:294-296) filters stageTrackerLinkedDrawnHere && !stageTrackerLinkedSupersededGone, so a closed PR without an open superseder (tracker-words-gate.js:86-89) still drops out — matching the new "a closed PR drawn elsewhere drives nothing" assertion. Returning the original elsewhere entry (not the clone) keeps drawn_under intact for the "under #M" chip.
  • tracker-row-render.js:106-114: prTargetKey/prRefs are only set inside the linked.length === 1 guard, so the secondary's PR column stays empty and stageTrackerLinkedElsewhereHtml (tracker-row-layout.js:290-292) still draws the chip — the test asserts exactly that.
  • Motion (tracker-motion.js:490-491) and list order (stageTrackerLead, tracker-row-layout.js:351-355) use the same rule, so lane, animation and sort agree as the body says.
  • via_pr consumers that resolve the PR by drawn_under — stageTrackerExternalPRWork (tracker-row.js:216-222) and the PR-author lookup at tracker-row.js:306-310 — find nothing for a secondary and fall back to {}/null rather than throwing. Net effect: the secondary row won't name the PR's external author in actor tips. Minor and arguably right (that belongs on the primary row); noting it so nobody is surprised.

Body matches the diff; changelog fragment present; index.html regenerated in step with static-src.

Only housekeeping: the rollup shows an overlayfs-exec-guard run as CANCELLED on this head — likely superseded by a re-push, worth a rerun if it doesn't settle green.

@hivecommons-hive hivecommons-hive Bot 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.

Read the diff against the tree at this head plus the callers of stageTrackerLinkedSorted. The core fix is sound; one consumer was left behind and now produces a wrong ownership string on the secondary row.

What checks out

  • stageTrackerLinkedRiding (tracker-row-layout.js:307-312) only fires when stageTrackerLinkedSorted is empty, filters to PRs drawn elsewhere that are not superseded-gone, and reuses stageTrackerLinkedSorted's comparator by clearing drawn_under on copies — so the lane, stageTrackerColMap (tracker-motion.js:490-491) and stageTrackerLead (tracker-row-layout.js:353-354) pick the same PR by the same order.
  • stageTrackerRowHtml (tracker-row-render.js:107-116) rides stage/merge_gate/caption from the PR but only fills prRefs/prTargetKey when linked.length === 1, so the PR column stays empty and the "under #M" chip still names it, as the body says.
  • The typeof … === 'function' guards match the existing harness convention in this file set.

Finding (low): stageTrackerExternalPRWork now reports "owner unknown" on the secondary row

tracker-row.js:217-222: for a via_pr row it filters linked_prs with stageTrackerLinkedDrawnHere, and sets source = linked.length === 1 ? linked[0] : {}. Before this PR a secondary row was never via_pr, so the function returned null at :218. Now the secondary row is via_pr, the drawn-here filter yields nothing, source = {}, and stageTrackerReturnedAuthor({}) (tracker-words.js:539) returns {author:'', kind:''} — which is neither hive nor bot, so the function returns a truthy object.

Consequence, when the row has no stage actor (sa.kind === STAGE_TRACKER_ACTOR_NONE): the actor tip at tracker-row.js:237 reads "PR ownership and current work activity are unknown", and the red-verdict lane at :326 reads "Fix owed · owner unknown" — while the primary row of the same hive-authored PR resolves source = linked[0], sees kind === 'hive', returns null, and reads "nobody on it". Two rows, one PR, two different owners. The fix is a one-liner: let stageTrackerExternalPRWork fall back to the ridden PR when nothing is drawn here (the same elsewhere filter stageTrackerLinkedRiding uses, without needing stages if you take the single remaining PR or thread stages through). stageTrackerReturnedAuthor (tracker-words.js:536) has the same drawn-here-only filter and would benefit from the same fallback.

Not blocking on my side — flagging for the author; a maintainer should decide whether to land as-is and follow up. I have not approved or requested changes.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@clubanderson
clubanderson merged commit 2fd05e6 into v5 Nov 9, 2026
96 of 104 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the fix/shared-pr-secondary-row-lane branch November 9, 2026 01:39
@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Activity monitor: secondary issue of a shared PR shows soaking in PR-in-progress lane instead of Merge queue

1 participant