Skip to content

Enable depends-on for GitHub Actions#4

Closed
saksham-chawla wants to merge 1 commit intocodex/replay-pr-201-basefrom
codex/replay-pr-201-head
Closed

Enable depends-on for GitHub Actions#4
saksham-chawla wants to merge 1 commit intocodex/replay-pr-201-basefrom
codex/replay-pr-201-head

Conversation

@saksham-chawla
Copy link
Collaborator

@saksham-chawla saksham-chawla commented Mar 18, 2026

With this we enable, that Pull-Requests will be build upon each other with a rebase.

Depends-On-NOT: eclipse-score#203


Enable depends-on for GitHub Actions

This pull request introduces a new GitHub Action that enables automatic rebasing of a pull request onto its declared dependencies. By adding Depends-On: #<PR_NUMBER> to a PR description, the CI workflow will fetch and rebase the current branch on the specified dependency PRs, ensuring that stacked PRs are always built on top of the latest changes from their predecessors. This streamlines the development process for interdependent features.

Architecture Diagram:

flowchart TD
    subgraph CI_Workflows
        W1["build_and_test_host.yml (+4 lines)"]
        W2["thread_sanitizer.yml (+4 lines)"]
        W3["address_undefined_behavior_leak_sanitizer.yml (+4 lines)"]
        W4["coverage_report.yml (+4 lines)"]
        W5["build_and_test_qnx.yml (+3 lines)"]
    end

    subgraph GitHub_Actions_Layer
        A["Checkout Repository (+2 lines)"]
        B["Rebase on Dependency PR (Composite Action) (+97 lines)"]
    end

    subgraph Script_Layer
        C["find_dependencies.py (+110 lines)"]
        D["rebase_on_prs.py (+164 lines)"]
    end

    subgraph Testing_Layer
        E["find_dependencies_test.py (+262 lines)"]
        F["rebase_on_prs_test.py (+355 lines)"]
    end

    subgraph External_Tools
        G[/"GitHub CLI"/]
        H[/"Git"/]
    end

    W1 --> A: "step"
    W2 --> A: "step"
    W3 --> A: "step"
    W4 --> A: "step"
    W5 --> A: "step"

    A --> B: "next step"

    B --> C: "runs script"
    B --> D: "runs script"

    C --> G: "uses"
    D --> G: "uses"
    D --> H: "uses"

    C -- "tested by" --> E
    D -- "tested by" --> F
Loading

This summary was automatically generated by @propel-code-bot

With this we enable, that Pull-Requests will be build upon each other with a rebase.
saksham-chawla pushed a commit that referenced this pull request Mar 18, 2026
…_implementation_Feedback_Review

Make GenericSkeleton and supporting types publicly accessible via types.h
@saksham-chawla saksham-chawla marked this pull request as ready for review March 18, 2026 18:41
Comment on lines +64 to +69
DEPENDENCY_PRS: ${{ steps.find-dependency.outputs.dependency_prs || 0 }}
DEPENDENCY_COUNT: ${{ steps.find-dependency.outputs.dependency_count || 0}}
REBASED: ${{ steps.rebase.outputs.rebased || 0 }}
REBASED_COUNT: ${{ steps.rebase.outputs.rebased_count || 0 }}
SKIPPED_PRS: ${{ steps.rebase.outputs.skipped_prs || 0}}
FAILED_PRS: ${{ steps.rebase.outputs.failed_prs || 0}}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Logic] Fallback || 0 causes the Summary step to display incorrect output when no dependencies are found. When dependency_prs is empty, ${{ '' || 0 }} evaluates to "0" (not empty string). This means:

  1. if [[ -z "$DEPENDENCY_PRS" ]] is false ("0" is non-empty), so the else branch runs, showing "Found 0 dependency PR(s): 0".
  2. if [[ -n "$SKIPPED_PRS" ]] and if [[ -n "$FAILED_PRS" ]] are always true when the rebase step is skipped, showing "Skipped PRs: 0" and "Failed: 0".

Use empty-string fallbacks instead:

      env:
        DEPENDENCY_PRS: ${{ steps.find-dependency.outputs.dependency_prs || '' }}
        DEPENDENCY_COUNT: ${{ steps.find-dependency.outputs.dependency_count || '0' }}
        REBASED: ${{ steps.rebase.outputs.rebased || '' }}
        REBASED_COUNT: ${{ steps.rebase.outputs.rebased_count || '0' }}
        SKIPPED_PRS: ${{ steps.rebase.outputs.skipped_prs || '' }}
        FAILED_PRS: ${{ steps.rebase.outputs.failed_prs || '' }}
Context for Agents
Fallback `|| 0` causes the Summary step to display incorrect output when no dependencies are found. When `dependency_prs` is empty, `${{ '' || 0 }}` evaluates to `"0"` (not empty string). This means:

1. `if [[ -z "$DEPENDENCY_PRS" ]]` is false ("0" is non-empty), so the `else` branch runs, showing "Found **0** dependency PR(s): 0".
2. `if [[ -n "$SKIPPED_PRS" ]]` and `if [[ -n "$FAILED_PRS" ]]` are always true when the rebase step is skipped, showing "Skipped PRs: 0" and "Failed: 0".

Use empty-string fallbacks instead:
```yaml
      env:
        DEPENDENCY_PRS: ${{ steps.find-dependency.outputs.dependency_prs || '' }}
        DEPENDENCY_COUNT: ${{ steps.find-dependency.outputs.dependency_count || '0' }}
        REBASED: ${{ steps.rebase.outputs.rebased || '' }}
        REBASED_COUNT: ${{ steps.rebase.outputs.rebased_count || '0' }}
        SKIPPED_PRS: ${{ steps.rebase.outputs.skipped_prs || '' }}
        FAILED_PRS: ${{ steps.rebase.outputs.failed_prs || '' }}
```

File: actions/rebase_on_dependency/action.yml
Line: 69
Chain of Thought
  • Validation Decision: accepted
  • Confidence: 0.97
  • Likelihood Score: 5/5
  • Likelihood Reason: This will inevitably produce incorrect summary output every time no dependencies are found (the common case), since the || 0 fallback makes DEPENDENCY_PRS equal to "0" instead of empty, causing the bash -z check to fail and the else branch to display misleading information.
  • Suggested Action: keep
  • Tool Calls Used: 2

Reasoning

The comment correctly identifies a real bug in newly introduced code (the file didn't exist before this PR).

Claim 1: ${{ &#39;&#39; || 0 }} evaluates to &#34;0&#34; — Verified. GitHub Actions documentation confirms that empty strings are falsy in expressions (GitHub Enterprise docs: "falsy values (false, 0, -0, "", '', null) are coerced to false"). When steps.find-dependency.outputs.dependency_prs is empty/unset, the || operator returns the right-hand side 0, which is then stringified to &#34;0&#34; when assigned to an environment variable.

Claim 2: -z &#34;$DEPENDENCY_PRS&#34; is false — Correct. In bash, -z checks for an empty string. &#34;0&#34; is non-empty, so the check fails and the else branch on line 78 executes.

Claim 3: The else branch shows "Found 0 dependency PR(s): 0" — Correct. Line 79 would output Found **0** dependency PR(s): 0 since both DEPENDENCY_COUNT and DEPENDENCY_PRS are &#34;0&#34;.

Claim 4: SKIPPED_PRS and FAILED_PRS -n checks are always true when rebase step is skipped — Correct. When the rebase step is skipped (line 52's if condition is false because dependency_prs is empty), all rebase outputs are unset. The || 0 fallback turns them into &#34;0&#34;, which is non-empty, making -n checks on lines 86 and 90 return true. This would incorrectly display "Skipped PRs: 0" and "Failed: 0".

The suggested fix is correct and specific: use || &#39;&#39; for list/string-type outputs that are checked with -z/-n (DEPENDENCY_PRS, REBASED, SKIPPED_PRS, FAILED_PRS), and || &#39;0&#39; for numeric counts (DEPENDENCY_COUNT, REBASED_COUNT). This preserves the correct behavior for all bash conditionals in the Summary step.

This is new code introduced by this PR, the bug is real, and the fix is concrete and actionable.

Evidence Summary

1. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/action.yml
1: # *******************************************************************************
2: # Copyright (c) 2026 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

Fix in Cursor

Comment on lines +70 to +72
dependency_state = get_pr_field(pr_number, "state")

if dependency_state == "MERGED":

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Logic] The rebase_on_pr function fetches headRefName first (which can succeed even for MERGED/CLOSED PRs) and only then checks state. If the gh CLI call for state fails (returns empty), the code falls through to the fetch+rebase path instead of failing or skipping. An empty dependency_state should be handled explicitly.

Suggested change
dependency_state = get_pr_field(pr_number, "state")
if dependency_state == "MERGED":
dependency_state = get_pr_field(pr_number, "state")
if not dependency_state:
print(f"::error::Could not fetch state for PR #{pr_number}.")
return REBASE_FAILED
if dependency_state == "MERGED":
Context for Agents
The `rebase_on_pr` function fetches `headRefName` first (which can succeed even for MERGED/CLOSED PRs) and only then checks `state`. If the `gh` CLI call for `state` fails (returns empty), the code falls through to the fetch+rebase path instead of failing or skipping. An empty `dependency_state` should be handled explicitly.

```suggestion
    dependency_state = get_pr_field(pr_number, "state")

    if not dependency_state:
        print(f"::error::Could not fetch state for PR #{pr_number}.")
        return REBASE_FAILED

    if dependency_state == "MERGED":
```

File: actions/rebase_on_dependency/scripts/rebase_on_prs.py
Line: 72
Chain of Thought
  • Validation Decision: accepted
  • Confidence: 0.95
  • Likelihood Score: 3/5
  • Likelihood Reason: The gh CLI call for state could fail due to transient network errors, rate limiting, or authentication issues in a CI environment, causing the code to silently fall through to the fetch+rebase path. While the headRefName guard catches some failure modes (e.g., PR doesn't exist), it does not cover the case where the first gh call succeeds but the second fails.
  • Suggested Action: keep
  • Tool Calls Used: 2

Reasoning

The comment is accurate and actionable:

  1. Accurate analysis: Looking at rebase_on_pr() (lines 47-103), the function first fetches headRefName (line 60) and checks for empty result (line 61). Then it fetches state (line 70) but does NOT check for an empty result. If get_pr_field(pr_number, &#34;state&#34;) returns &#34;&#34; (which happens when gh CLI returns non-zero, per line 42-43), then:

    • dependency_state == &#34;MERGED&#34; → False
    • dependency_state == &#34;CLOSED&#34; → False
    • Code falls through to the fetch+rebase path (lines 86+)
  2. Real gap: The headRefName check on line 61 only guards against the PR not existing or the first CLI call failing. If the first call succeeds but the second fails (transient error, rate limit hit, etc.), the empty state is silently ignored and the code proceeds to rebase without knowing whether the PR is merged/closed.

  3. Actionable: The suggestion provides a concrete code fix — check for empty dependency_state and return REBASE_FAILED with an error message.

  4. Introduced by this PR: The file is entirely new (all lines are added), so this is clearly within the PR's scope.

  5. No existing tests cover this case: Looking at the test file, there's no test for the scenario where headRefName succeeds but state fails. All test mocks either return valid states or fail both calls together.

Evidence Summary

1. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/test/rebase_on_prs_test.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

2. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/rebase_on_prs.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

Fix in Cursor

capture_output=True,
text=True,
)
if result.returncode != 0:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Reliability] The gh CLI command can fail for reasons other than the PR not existing (e.g., auth issues, network problems). Silently returning an empty string hides these errors, making debugging difficult. It would be more robust to raise an exception here, similar to get_pr_body in find_dependencies.py, and let the caller handle it.

Suggested change
if result.returncode != 0:
if result.returncode != 0:
raise RuntimeError(
f"gh CLI failed to get field '{field}' for PR #{pr_number} (exit {result.returncode}): {result.stderr.strip()}"
)

The calling function rebase_on_pr would then need a try...except block to handle this potential RuntimeError.

Context for Agents
The `gh` CLI command can fail for reasons other than the PR not existing (e.g., auth issues, network problems). Silently returning an empty string hides these errors, making debugging difficult. It would be more robust to raise an exception here, similar to `get_pr_body` in `find_dependencies.py`, and let the caller handle it.

```suggestion
    if result.returncode != 0:
        raise RuntimeError(
            f"gh CLI failed to get field '{field}' for PR #{pr_number} (exit {result.returncode}): {result.stderr.strip()}"
        )
```
The calling function `rebase_on_pr` would then need a `try...except` block to handle this potential `RuntimeError`.

File: actions/rebase_on_dependency/scripts/rebase_on_prs.py
Line: 42
Chain of Thought
  • Validation Decision: accepted
  • Confidence: 0.88
  • Likelihood Score: 3/5
  • Likelihood Reason: Auth/network failures with the gh CLI are plausible in CI environments. When they occur, the empty-string return produces misleading errors (first call) or silently proceeds with an incorrect empty state (second call), potentially causing confusing downstream behavior.
  • Suggested Action: keep
  • Tool Calls Used: 2

Reasoning

The comment makes three claims, all of which are accurate:

  1. Silent error swallowing: get_pr_field (lines 42-43) returns &#34;&#34; on any non-zero exit from gh, hiding the actual reason for failure (auth, network, rate-limiting, etc.).

  2. Inconsistency with get_pr_body: The sibling function get_pr_body in find_dependencies.py (lines 53-56) raises RuntimeError on gh CLI failure, with its caller using a proper try...except block (lines 84-89). This is a legitimate internal inconsistency in the same PR's newly introduced code.

  3. Concrete downstream impact: Looking at the callers:

    • Line 60: get_pr_field(pr_number, &#34;headRefName&#34;) — on failure returns &#34;&#34;, triggering the misleading error "Make sure the PR exists and is open" (when the real issue might be auth/network). At least it does return REBASE_FAILED.
    • Line 70: get_pr_field(pr_number, &#34;state&#34;) — on failure returns &#34;&#34;, which doesn't match &#34;MERGED&#34; or &#34;CLOSED&#34;, so the code silently proceeds to attempt git fetch/rebase. This is more problematic, as a transient network error could cause the code to attempt operations on a PR it has no valid state for.

The comment is actionable with a concrete code suggestion and an explanation of what the caller needs. This is newly introduced code in this PR, not legacy debt. The inconsistency with get_pr_body is a valid design concern for code written in the same PR.

Evidence Summary

1. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/rebase_on_prs.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

2. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/find_dependencies.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

Fix in Cursor

Comment on lines 103 to +108
with:
ref: ${{ github.head_ref || github.event.pull_request.head.ref || github.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
fetch-depth: 0 # Required for rebasing
- name: Rebase on dependency PR
uses: ./actions/rebase_on_dependency

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Reliability] In this workflow we clone ${{ github.event.pull_request.head.repo.full_name }} for forked PRs so the remote origin points at the contributor’s fork. The rebase action that follows always fetches dependency heads via git fetch origin refs/pull/<n>/head:…, but GitHub exposes the refs/pull/* namespace only on the base repository, never on contributor forks. As a result, git fetch fails for every forked PR that declares dependencies, and the entire job aborts before any build happens. To keep the rebase step working for forks, either check out the base repository as origin (and fetch the fork separately) or add a second remote pointing to ${{ github.repository }} and make the action fetch from that remote instead.

Context for Agents
In this workflow we clone `${{ github.event.pull_request.head.repo.full_name }}` for forked PRs so the remote `origin` points at the contributor’s fork. The rebase action that follows always fetches dependency heads via `git fetch origin refs/pull/<n>/head:…`, but GitHub exposes the `refs/pull/*` namespace only on the base repository, never on contributor forks. As a result, `git fetch` fails for every forked PR that declares dependencies, and the entire job aborts before any build happens. To keep the rebase step working for forks, either check out the base repository as `origin` (and fetch the fork separately) or add a second remote pointing to `${{ github.repository }}` and make the action fetch from that remote instead.


File: .github/workflows/build_and_test_qnx.yml
Line: 108
Chain of Thought
  • Validation Decision: accepted
  • Confidence: 0.95
  • Likelihood Score: 4/5
  • Likelihood Reason: The QNX workflow is explicitly designed to support forked PRs (pull_request_target trigger, fork-specific label gate). Whenever a forked PR declares a Depends-On dependency, the git fetch will deterministically fail because refs/pull/* doesn't exist on the fork's origin.
  • Suggested Action: keep
  • Tool Calls Used: 6

Reasoning

The comment accurately identifies a real bug introduced by this PR. Here's the chain:

  1. The QNX workflow uses pull_request_target and explicitly checks out the fork repo: repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}. For forked PRs, origin points to the contributor's fork.

  2. The newly-added rebase step (line 107-108) calls rebase_on_prs.py, which on line 90 runs git fetch origin refs/pull/{pr_number}/head:{local_ref}.

  3. GitHub's refs/pull/&lt;n&gt;/head refs are only available on the base repository where the PR was opened, not on contributor forks. So fetching from origin (the fork) will fail.

  4. The QNX workflow has explicit fork-handling logic (precheck job with test-qnx label gate), confirming this is an actively-used fork PR path.

  5. In contrast, build_and_test_host.yml uses the default checkout without a repository: override, so origin naturally points to the base repo — no issue there.

The comment is actionable, providing two concrete solutions: (a) check out the base repository as origin and fetch the fork separately, or (b) add a second remote pointing to the base repo. The irony is that rebase_on_prs.py even has a comment at line 83-85 claiming "fork PRs are supported" — but that's only true when origin points to the base repo.

Evidence Summary

1. File Snapshot (HEAD)

  • File: .github/workflows/build_and_test_qnx.yml
1: # *******************************************************************************
2: # Copyright (c) 2024 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

2. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/find_dependencies.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

3. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/rebase_on_prs.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

4. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/action.yml
1: # *******************************************************************************
2: # Copyright (c) 2026 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

5. File Snapshot (HEAD)

  • File: .github/workflows/build_and_test_host.yml
  • Lines: 90-120
63:

6. File Snapshot (HEAD)

  • File: .github/workflows/build_and_test_host.yml
1: # *******************************************************************************
2: # Copyright (c) 2024 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

Fix in Cursor

Comment on lines +201 to +208
def test_merge_group_event_runs(self):
code, output = self._run_main(
{"EVENT_NAME": "merge_group", "PR_NUMBER": "123"},
pr_body="Depends-On: #456",
)
self.assertEqual(code, 0)
self.assertIn("dependency_prs=456\n", output)
self.assertIn("dependency_count=1\n", output)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

[Testing] Test test_merge_group_event_runs will fail: merge_group is not in the allowed event names in main() ("pull_request", "pull_request_target", "workflow_run"). The code will take the early-exit path writing empty outputs, so the assertions assertIn("dependency_prs=456\n", output) and assertIn("dependency_count=1\n", output) will not hold.

Either add "merge_group" to the allowed events in find_dependencies.py:

if event_name not in ("pull_request", "pull_request_target", "workflow_run", "merge_group"):

Or fix the test expectations to match the current skip behaviour (empty outputs, code 0). Note that in real CI, github.event.pull_request.number is empty for merge_group events, so even adding it to the list would result in the PR number check exiting early.

Context for Agents
Test `test_merge_group_event_runs` will fail: `merge_group` is not in the allowed event names in `main()` (`"pull_request"`, `"pull_request_target"`, `"workflow_run"`). The code will take the early-exit path writing empty outputs, so the assertions `assertIn("dependency_prs=456\n", output)` and `assertIn("dependency_count=1\n", output)` will not hold.

Either add `"merge_group"` to the allowed events in `find_dependencies.py`:
```python
if event_name not in ("pull_request", "pull_request_target", "workflow_run", "merge_group"):
```
Or fix the test expectations to match the current skip behaviour (empty outputs, code 0). Note that in real CI, `github.event.pull_request.number` is empty for `merge_group` events, so even adding it to the list would result in the PR number check exiting early.

File: actions/rebase_on_dependency/test/find_dependencies_test.py
Line: 208
Chain of Thought
  • Validation Decision: accepted
  • Confidence: 0.97
  • Likelihood Score: 5/5
  • Likelihood Reason: The test will inevitably fail. The code path is deterministic: "merge_group" is not in the allowed event names tuple, so main() will always take the early-exit path and write empty outputs, causing both assertIn checks to fail.
  • Suggested Action: keep
  • Tool Calls Used: 1

Reasoning

The comment correctly identifies a test-code mismatch introduced by this PR:

  1. find_dependencies.py line 72 checks: if event_name not in (&#34;pull_request&#34;, &#34;pull_request_target&#34;, &#34;workflow_run&#34;). The string &#34;merge_group&#34; is NOT in this tuple.

  2. The test test_merge_group_event_runs (lines 201-208) sets EVENT_NAME to &#34;merge_group&#34; and expects the output to contain &#34;dependency_prs=456\n&#34; and &#34;dependency_count=1\n&#34;.

  3. Since &#34;merge_group&#34; fails the event name check, main() takes the early-exit path at lines 73-75, printing "Not a pull request event" and writing empty outputs (dependency_prs=\n and dependency_count=0\n).

  4. The test assertions assertIn(&#34;dependency_prs=456\n&#34;, output) and assertIn(&#34;dependency_count=1\n&#34;, output) will definitely fail.

The comment is accurate, well-explained, and provides two concrete fix options: either add &#34;merge_group&#34; to the allowed events in the source code, or update the test expectations to match the current skip behavior. Both the test and source file are new/modified in this PR, so this is clearly an issue introduced by this PR.

Evidence Summary

1. File Snapshot (HEAD)

  • File: actions/rebase_on_dependency/scripts/find_dependencies.py
1: #!/usr/bin/env python3
2: 
3: # *******************************************************************************
4: # Copyright (c) 2026 Contributors to the Eclipse Foundation
5: #
6: # See the NOTICE file(s) distributed with this work for additional
7: # information regarding copyright ownership.
8: #
9: # This program and the accompanying materials are made available under the
10: # terms of the Apache License Version 2.0 which is available at
...

Fix in Cursor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants