Skip to content

Commit 1857f9b

Browse files
committed
Fix comment not being removed if preflight has no match
1 parent 5c0453f commit 1857f9b

6 files changed

Lines changed: 31 additions & 44 deletions

File tree

README.md

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ For every modified PHP file the action:
1010
2. Snapshots the same files on the **base** ref (extracted with `git archive`).
1111
3. Diffs the two snapshots. A method or constant is only reported on a class when its **introduction point** is the class itself — implementations of existing interface methods, overrides of parent methods, or members already declared on a parent in the unchanged code are not flagged.
1212

13-
The result is written to a directory containing `comment-body.txt` (a markdown body, possibly empty) and `pr-number.txt`. A companion `scripts/post-comment.sh` posts/updates/deletes a PR comment based on that artifact.
13+
The result is written to a directory containing `comment-body.txt` (a markdown body, possibly empty). A companion `post-comment` sub-action posts, updates, or deletes a PR comment based on that body.
1414

1515
## Why static reflection?
1616

@@ -84,8 +84,10 @@ jobs:
8484
echo "should-run=true" >> "$GITHUB_OUTPUT"
8585
fi
8686
87+
# Resolve the PR unconditionally so we can clean up a stale comment
88+
# even when preflight says no API-relevant changes (e.g. a PR was
89+
# updated to revert previously-reported additions).
8790
- id: pr
88-
if: steps.preflight.outputs.should-run == 'true'
8991
env:
9092
GH_TOKEN: ${{ github.token }}
9193
run: |
@@ -96,21 +98,20 @@ jobs:
9698
echo "number=$number" >> "$GITHUB_OUTPUT"
9799
echo "base-ref=$base_ref" >> "$GITHUB_OUTPUT"
98100
99-
- if: steps.pr.outputs.number
101+
- if: steps.pr.outputs.number && steps.preflight.outputs.should-run == 'true'
100102
uses: actions/checkout@v6
101103
with:
102104
ref: refs/pull/${{ steps.pr.outputs.number }}/head
103105
fetch-depth: 0
104106
persist-credentials: false
105107
106-
- if: steps.pr.outputs.number
108+
- if: steps.pr.outputs.number && steps.preflight.outputs.should-run == 'true'
107109
run: git fetch --no-tags origin "${{ steps.pr.outputs.base-ref }}:refs/remotes/origin/${{ steps.pr.outputs.base-ref }}"
108110
109-
- if: steps.pr.outputs.number
111+
- if: steps.pr.outputs.number && steps.preflight.outputs.should-run == 'true'
110112
uses: composer/api-surface-check@main
111113
with:
112114
base-ref: origin/${{ steps.pr.outputs.base-ref }}
113-
pr-number: ${{ steps.pr.outputs.number }}
114115
# INPUTS / CONFIG GOES HERE
115116
# install-dependencies: true
116117
# paths: src/**/*.php
@@ -119,10 +120,15 @@ jobs:
119120
# show-removed: true
120121
# show-modified: true
121122
123+
# Always run post-comment when we have a PR. With analysis skipped,
124+
# comment-body.txt is missing/empty and post-comment deletes any
125+
# previously-posted bot comment.
122126
- if: steps.pr.outputs.number
127+
uses: composer/api-surface-check/post-comment@main
128+
with:
129+
pr-number: ${{ steps.pr.outputs.number }}
123130
env:
124131
GH_TOKEN: ${{ github.token }}
125-
uses: composer/api-surface-check/post-comment@main
126132
```
127133

128134
### Why two workflows
@@ -157,7 +163,6 @@ jobs:
157163
| Input | Default | Description |
158164
| ---------------- | -------------------------- | ------------------------------------------------------------ |
159165
| `base-ref` | _required_ | Git ref to compare HEAD against (e.g. `origin/main`). |
160-
| `pr-number` | (empty) | Saved alongside the comment body for the comment workflow. |
161166
| `output-dir` | `api-surface-result` | Directory the artifact is written to. |
162167
| `comment-marker` | `<!-- api-surface-bot -->` | HTML marker used to identify previous bot comments. |
163168

@@ -184,12 +189,13 @@ Quickly scans the PR diff for tokens that could affect the API surface. Writes `
184189

185190
### `composer/api-surface-check/post-comment`
186191

187-
Posts, updates, or deletes a PR comment based on the artifact produced by the main action. Use it in the comment workflow.
192+
Posts, updates, or deletes a PR comment based on the artifact produced by the main action. Use it in the comment workflow. Run it unconditionally once the PR is resolved — when given an empty/missing `comment-body.txt` it deletes any previously-posted bot comment, which fixes stale comments after a PR is updated to revert API additions.
188193

189-
| Input | Default | Description |
190-
| ---------------- | ------------------------------------ | -------------------------------------------------------- |
191-
| `output-dir` | `api-surface-result` | Directory containing `comment-body.txt`/`pr-number.txt`. |
192-
| `comment-marker` | `<!-- api-surface-bot -->` | HTML marker that identifies previous bot comments. |
194+
| Input | Default | Description |
195+
| ---------------- | -------------------------- | ---------------------------------------------------------------------------------------------------------- |
196+
| `pr-number` | _required_ | PR number to post / update / delete the comment on. |
197+
| `output-dir` | `api-surface-result` | Directory containing `comment-body.txt`. An empty/missing body deletes any existing marked comment. |
198+
| `comment-marker` | `<!-- api-surface-bot -->` | HTML marker that identifies previous bot comments. |
193199

194200
Requires `GH_TOKEN` env (passed in by the consumer) with `pull-requests: write`.
195201

action.yml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@ inputs:
2121
description: 'Directory to write outputs to (relative to the consuming workflow''s working dir).'
2222
required: false
2323
default: 'api-surface-result'
24-
pr-number:
25-
description: 'Pull request number; written to the artifact for the comment workflow.'
26-
required: false
27-
default: ''
2824
include-internal:
2925
description: 'Include symbols marked with @internal or @private.'
3026
required: false
@@ -71,7 +67,7 @@ outputs:
7167
description: 'Whether any reportable API surface changes were detected.'
7268
value: ${{ steps.detect.outputs.has-changes }}
7369
output-dir:
74-
description: 'Directory containing comment-body.txt, pr-number.txt, and snapshots.'
70+
description: 'Directory containing comment-body.txt and the head/base snapshot JSON files.'
7571
value: ${{ steps.detect.outputs.output-dir }}
7672

7773
runs:
@@ -120,7 +116,6 @@ runs:
120116
SOURCE_ROOTS: ${{ inputs.source-roots }}
121117
VENDOR_PATH: ${{ steps.project-deps.outputs.vendor-path }}
122118
OUTPUT_DIR: ${{ inputs.output-dir }}
123-
PR_NUMBER: ${{ inputs.pr-number }}
124119
INCLUDE_INTERNAL: ${{ inputs.include-internal }}
125120
TYPES: ${{ inputs.types }}
126121
VISIBILITY: ${{ inputs.visibility }}

post-comment/action.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ branding:
66
color: 'blue'
77

88
inputs:
9+
pr-number:
10+
description: 'PR number to post / update / delete the comment on.'
11+
required: true
912
output-dir:
10-
description: 'Directory containing comment-body.txt and pr-number.txt produced by the analysis action.'
13+
description: 'Directory containing the comment-body.txt produced by the analysis action. An empty or missing body deletes any existing marked comment (used for stale-comment cleanup when analysis is skipped).'
1114
required: false
1215
default: 'api-surface-result'
1316
comment-marker:
@@ -23,4 +26,5 @@ runs:
2326
env:
2427
OUTPUT_DIR: ${{ inputs.output-dir }}
2528
COMMENT_MARKER: ${{ inputs.comment-marker }}
29+
PR_NUMBER: ${{ inputs.pr-number }}
2630
run: bash ${{ github.action_path }}/../scripts/post-comment.sh

scripts/detect.sh

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#!/usr/bin/env bash
22
# Detect API surface changes between BASE_REF and HEAD.
33
# Reads configuration from env vars (set by the action), produces an output
4-
# directory containing comment-body.txt (markdown body, possibly empty),
5-
# pr-number.txt (if PR_NUMBER is set), and the raw snapshots/diff for debugging.
4+
# directory containing comment-body.txt (markdown body, possibly empty) and
5+
# the head/base snapshot JSON files (kept for debugging).
66
set -euo pipefail
77

88
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
@@ -12,7 +12,6 @@ BASE_REF="${BASE_REF:?BASE_REF is required}"
1212
PATHS_GLOB="${PATHS_GLOB:-src/**/*.php}"
1313
SOURCE_ROOTS="${SOURCE_ROOTS:-src}"
1414
OUTPUT_DIR="${OUTPUT_DIR:-api-surface-result}"
15-
PR_NUMBER="${PR_NUMBER:-}"
1615

1716
INCLUDE_INTERNAL="${INCLUDE_INTERNAL:-false}"
1817
TYPES="${TYPES:-class,interface,trait,enum,method,property,constant}"
@@ -38,9 +37,6 @@ read -r -a roots_array <<< "$(echo "${SOURCE_ROOTS}" | tr '\n' ' ' | tr ',' ' ')
3837

3938
mkdir -p "${OUTPUT_DIR}"
4039
: > "${OUTPUT_DIR}/comment-body.txt"
41-
if [[ -n "${PR_NUMBER}" ]]; then
42-
echo "${PR_NUMBER}" > "${OUTPUT_DIR}/pr-number.txt"
43-
fi
4440

4541
# All files in scope (added, modified, or deleted) — snapshot.php will silently
4642
# ignore missing files, so we can pass the same list for both refs.

scripts/post-comment.sh

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,11 @@ OUTPUT_DIR="${OUTPUT_DIR:-api-surface-result}"
88
COMMENT_MARKER="${COMMENT_MARKER:-<!-- api-surface-bot -->}"
99
GITHUB_REPOSITORY="${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}"
1010
: "${GH_TOKEN:?GH_TOKEN is required}"
11+
: "${PR_NUMBER:?PR_NUMBER is required}"
1112

12-
if [[ ! -f "${OUTPUT_DIR}/pr-number.txt" ]]; then
13-
echo "No pr-number.txt in ${OUTPUT_DIR}; nothing to do."
14-
exit 0
15-
fi
16-
17-
PR_NUMBER=$(cat "${OUTPUT_DIR}/pr-number.txt")
13+
# An empty body (or missing comment-body.txt) means: delete any existing
14+
# marked comment. This is what cleans up stale comments after a PR is updated
15+
# to remove the API additions originally reported.
1816
COMMENT_BODY=""
1917
if [[ -s "${OUTPUT_DIR}/comment-body.txt" ]]; then
2018
COMMENT_BODY=$(cat "${OUTPUT_DIR}/comment-body.txt")

tests/DetectTest.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,18 +239,6 @@ public function testModifiedSignatureRequiresFlag(): void
239239
$this->assertStringContainsString('now: `public function mut(int $x, int $y): void`', $body);
240240
}
241241

242-
public function testPrNumberIsWrittenWhenProvided(): void
243-
{
244-
$this->commit(['src/Foo.php' => "<?php\nnamespace L;\nclass Foo {}\n"], 'base');
245-
$this->commit(['src/Foo.php' => "<?php\nnamespace L;\nclass Foo { public function hi(): void {} }\n"], 'add method');
246-
247-
$this->runDetect(['PR_NUMBER' => '424242']);
248-
249-
$prFile = $this->repoDir . '/result/pr-number.txt';
250-
$this->assertFileExists($prFile);
251-
$this->assertSame('424242', trim(file_get_contents($prFile)));
252-
}
253-
254242
public function testVendorParentMethodIsNotReportedWhenVendorPathIsProvided(): void
255243
{
256244
// Pretend vendor/ is composer-installed: it lives outside SOURCE_ROOTS

0 commit comments

Comments
 (0)