Skip to content

fix(extension): resolve sticky-offset overlap, Files-tab discovery, and missed PR-feedback fixes#401

Open
jd wants to merge 1 commit intodevs/jd/worktree-leverage-json/split-src-mergify-js-es-modules-esbuild-bundle--0ff58889from
devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-files-tab-discovery--7bf7efe8
Open

fix(extension): resolve sticky-offset overlap, Files-tab discovery, and missed PR-feedback fixes#401
jd wants to merge 1 commit intodevs/jd/worktree-leverage-json/split-src-mergify-js-es-modules-esbuild-bundle--0ff58889from
devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-files-tab-discovery--7bf7efe8

Conversation

@jd
Copy link
Copy Markdown
Member

@jd jd commented Apr 30, 2026

Six fixes against the Mergify Stacks feature:

  1. Negative caching in fetchCommentBodies: a 404/5xx from edit_form
    is now remembered for 60s so a deleted comment or a transient
    failure no longer causes a refetch on every tryInject tick.

  2. Stale UI removal in renderMergifyContext: the early-return paths
    for "no comment bodies" and "panel is null" now explicitly remove
    any existing #mergify-context and #mergify-stack-nav. Previously
    the panel could linger after the markers disappeared.

  3. In-flight dedup for status fetches: gatherPrStatuses now reuses a
    single Promise per (org/repo/num/head_sha) so multiple ticks
    during React reconciliation don't fire duplicate fetchPrStatus
    requests.

  4. Toolbar reset on null nav path: injectStackNav now resets
    flex-wrap and removes the padding-bottom override unconditionally,
    not gated on whether our nav node still exists. React can drop
    our node while leaving the inline styles behind.

  5. Sticky-offset overlap: GitHub's diff-view file-tree pane uses
    position: sticky; top: 60px hardcoded to the toolbar's original
    height. Our nav grows the toolbar to ~93px, which would otherwise
    hide the pane behind our nav. injectStackNav now writes a small

    <style> rule overriding the pane's top to the toolbar's actual current height (with !important to outweigh the class rule). The rule is removed when our nav is removed.
  6. Files-tab discovery fallback: on the /files tab GitHub doesn't
    render the conversation timeline at all, so the local DOM scan
    for Mergify-related comment IDs always returned empty and the
    panel/nav never showed. fetchCommentBodies now falls back to
    fetching the Conversation page HTML and parsing comment IDs from
    it. Result is cached per-PR with a 5-minute TTL so subsequent
    ticks don't re-fetch the ~500 KB page.

Six new tests cover each fix.

Depends-On: #398

@jd
Copy link
Copy Markdown
Member Author

jd commented Apr 30, 2026

This pull request is part of a Mergify stack:

# Pull Request Link
1 refactor(extension): split src/mergify.js into ES modules with esbuild bundle #398
2 fix(extension): resolve sticky-offset overlap, Files-tab discovery, and missed PR-feedback fixes #401 👈
3 chore(extension): drop per-tick debug logs that flood console on diff pages #402

@mergify mergify Bot had a problem deploying to Mergify Merge Protections April 30, 2026 13:13 Failure
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 30, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 Required Reviews

Waiting for:

  • #approved-reviews-by >= 2
This rule is failing.
  • any of:
    • #approved-reviews-by >= 2
    • author = dependabot[bot]

🟢 ⛓️ Depends-On Requirements

Wonderful, this rule succeeded.

Requirement based on the presence of Depends-On in the body of the pull request

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?:

🟢 🔎 Reviews

Wonderful, this rule succeeded.
  • #changes-requested-reviews-by = 0
  • #review-requested = 0
  • #review-threads-unresolved = 0

🟢 📕 PR description

Wonderful, this rule succeeded.
  • body ~= (?ms:.{48,})

@jd jd force-pushed the devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-files-tab-discovery--7bf7efe8 branch from f7853cb to 5718ad7 Compare April 30, 2026 14:02
@jd
Copy link
Copy Markdown
Member Author

jd commented Apr 30, 2026

Revision history

# Type Changes Reason Date
1 initial f7853cb 2026-04-30 14:02 UTC
2 content f7853cb → 5718ad7 2026-04-30 14:02 UTC
3 rebase 5718ad7 → 3e674b9 2026-05-04 09:37 UTC
4 rebase 3e674b9 → e93520e 2026-05-04 11:25 UTC

@mergify mergify Bot had a problem deploying to Mergify Merge Protections April 30, 2026 14:02 Failure
@jd jd marked this pull request as ready for review April 30, 2026 15:11
@jd jd force-pushed the devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-files-tab-discovery--7bf7efe8 branch from 5718ad7 to 3e674b9 Compare May 4, 2026 09:37
@mergify mergify Bot had a problem deploying to Mergify Merge Protections May 4, 2026 09:37 Failure
…nd missed PR-feedback fixes

Six fixes against the Mergify Stacks feature:

1. Negative caching in fetchCommentBodies: a 404/5xx from edit_form
   is now remembered for 60s so a deleted comment or a transient
   failure no longer causes a refetch on every tryInject tick.

2. Stale UI removal in renderMergifyContext: the early-return paths
   for "no comment bodies" and "panel is null" now explicitly remove
   any existing #mergify-context and #mergify-stack-nav. Previously
   the panel could linger after the markers disappeared.

3. In-flight dedup for status fetches: gatherPrStatuses now reuses a
   single Promise per (org/repo/num/head_sha) so multiple ticks
   during React reconciliation don't fire duplicate fetchPrStatus
   requests.

4. Toolbar reset on null nav path: injectStackNav now resets
   flex-wrap and removes the padding-bottom override unconditionally,
   not gated on whether our nav node still exists. React can drop
   our node while leaving the inline styles behind.

5. Sticky-offset overlap: GitHub's diff-view file-tree pane uses
   position: sticky; top: 60px hardcoded to the toolbar's original
   height. Our nav grows the toolbar to ~93px, which would otherwise
   hide the pane behind our nav. injectStackNav now writes a small
   <style> rule overriding the pane's top to the toolbar's actual
   current height (with !important to outweigh the class rule).
   The rule is removed when our nav is removed.

6. Files-tab discovery fallback: on the /files tab GitHub doesn't
   render the conversation timeline at all, so the local DOM scan
   for Mergify-related comment IDs always returned empty and the
   panel/nav never showed. fetchCommentBodies now falls back to
   fetching the Conversation page HTML and parsing comment IDs from
   it. Result is cached per-PR with a 5-minute TTL so subsequent
   ticks don't re-fetch the ~500 KB page.

Six new tests cover each fix.

Change-Id: I7bf7efe8f0e3b5dc3e0789c151fc9b89072a22b0
@jd jd force-pushed the devs/jd/worktree-leverage-json/resolve-sticky-offset-overlap-files-tab-discovery--7bf7efe8 branch from 3e674b9 to e93520e Compare May 4, 2026 11:25
@mergify mergify Bot had a problem deploying to Mergify Merge Protections May 4, 2026 11:26 Failure
@jd jd requested a review from a team May 5, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants