feat(workflows): add closed-PR comment redirect

[aidandaly24]

Description

Add a workflow that responds to comments on closed PRs by external users — posts a redirect asking them to open a new issue, and pages oncall via Slack so closed-PR comments are not missed.

Also hardens the existing slack-issue-notification.yml to use injection-safe env: + toJSON() and emits a uniform 20-key payload (with event_type discriminator) so the Slack workflow can branch on event type.

Why: COE AI-7 (Bedrock AgentCore SDK SPII OTEL leak). An external user reported the v1.4.8 SPII leak by commenting on a closed revert PR; oncall does not monitor closed-PR comments, so the report was missed for ~20 hours. This closes that detection gap.

Quip: https://quip-amazon.com/SmCwABMBwzgH

Related Issue

Documentation PR

N/A (workflow-only change, no user-facing docs)

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Other (please describe):

Testing

How have you tested the change?

• I ran `npm run test:unit` and `npm run test:integ`
• I ran `npm run typecheck`
• I ran `npm run lint`
• If I modified `src/assets/`, I ran `npm run test:update-snapshots` and committed the updated snapshots

Workflow-only change — no app-code tests apply. Manual test plan once merged:

• Open and close a test PR; comment as a non-maintainer; verify bot posts redirect comment AND Slack alert fires.
• Comment as a maintainer; verify no redirect, no alert.
• Open a real issue; verify existing Slack alert behavior (regression check).

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

Slack-side change (out of band)

The shared Slack workflow `GitHub Issue Response - Moab` was updated separately to:

• Add `event_type`, `pr_number`, `pr_title`, `pr_url`, `pr_author`, `pr_state`, `pr_closed_at`, `pr_merged_at`, `comment_id`, `comment_url`, `comment_author`, `comment_body` as trigger variables.
• Branch on `event_type` to render closed-PR-comment alerts differently from issue-opened alerts.
• Page `@moab-primary-oncalls` for closed-PR comments.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh release download pr-1328-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz

[agentcore-cli-automation]

Two correctness concerns around the dedup logic and oncall paging volume — see inline comments. The env/toJSON hardening on the existing notification workflow looks good.

[agentcore-cli-automation]

Slack notification is not deduped per PR/commenter, so oncall will be paged on every external comment.

The Post redirect comment step is gated by steps.existing.outputs.already_posted != 'true' (correct — we don't want to spam the PR with redirect replies), but Notify Slack is gated only on steps.perm.outputs.skip != 'true'. The result is:

• A back-and-forth thread from one external user on a closed PR pages oncall on every comment.
• A single user spamming a closed PR with N comments produces N pages.
• Once the redirect comment has been posted, subsequent comments still page (despite the redirect having explicitly told them to open an issue).

Given this workflow exists specifically to close a detection gap, some over-paging is probably acceptable, but unbounded paging from a single thread is not. Options:

1. Also gate Notify Slack on steps.existing.outputs.already_posted != 'true' — pages once per (PR, external commenter) cohort, but risks missing genuinely new info posted after the redirect.
2. Keep the current behavior on the GitHub side and dedup on the Slack workflow side (e.g., suppress alerts where comment_author already alerted on the same pr_number within N hours). This is mentioned as a side-channel change, so the hook is already there.
3. Track per-commenter dedup with a second marker comment scoped to the commenter login (similar to how existing works, but keyed on comment.user.login).

Whichever you pick, please document the chosen behavior in the comment block above this step — right now the comment says "Slack alert is the load-bearing part" but doesn't address repeat-comment volume.

[agentcore-cli-automation]

Missing concurrency control makes the marker-comment dedup racy.

The Check for existing redirect comment step relies on a marker (<!-- closed-pr-comment-redirect -->) being present in a previously posted comment. If two external comments arrive within the time it takes for the first workflow run to post its redirect (a few seconds), both runs will see no marker and both will post a redirect comment — defeating the dedup.

This is a realistic scenario: someone posts two follow-up comments back-to-back, or two different external users comment around the same time.

Suggest adding a per-PR concurrency group at the workflow or job level, e.g.:

concurrency:
  group: closed-pr-comment-${{ github.event.issue.number }}
  cancel-in-progress: false

This serializes runs for the same PR so the marker check is reliable. cancel-in-progress: false is important here — we still want the second run to execute (to page Slack for the second comment), just after the first has finished posting its marker.

[agentcore-cli-automation]

pr_state env var will be empty in the payload because of step ordering / env: evaluation.

The env: block on this step references ${{ steps.pr_state.outputs.state }}, which is fine — that step runs before this one. But please double-check by running the workflow once: in some env: evaluation paths, steps.<id>.outputs.<x> is only available within with:/run: and not in step-level env:. If it does come through empty, the fix is either:

1. Inline the expression directly in the payload: pr_state: "${{ steps.pr_state.outputs.state }}" (safe — values are merged/closed, not user input).
2. Promote pr_state to a job-level output and reference it via needs/jobs (overkill for a single job).

Not a blocker if you've verified it works end-to-end on a test repo, but worth confirming since the manual test plan is post-merge.

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	43.73%	9316 / 21299
🔵	Statements	42.98%	9884 / 22996
🔵	Functions	40.33%	1608 / 3987
🔵	Branches	40.45%	6058 / 14976

Generated in workflow #3168 for commit 57a2506 by the Vitest Coverage Report Action

[agentcore-devx-automation]

Claude Security Review: no high-confidence findings. (run)

[agentcore-cli-automation]

Reviewed the latest revision (57a2506). All three concerns from the prior automated review (Slack paging volume, marker-comment race, pr_state step-output evaluation) have been addressed in this commit:

• Notify Slack is now gated on steps.existing.outputs.already_posted != 'true', so paging is bounded to the first external comment per PR.
• A workflow-level concurrency group keyed on github.event.issue.number with cancel-in-progress: false makes the marker-comment dedup race-free.
• pr_state is inlined directly in the Slack payload ("${{ steps.pr_state.outputs.state }}"), avoiding any step-level env: evaluation ambiguity. The value is enum-typed (merged|closed), so injection-safe.

No new blocking issues found. Schemas in both workflows match (20 keys, event_type discriminator), attacker-controlled fields (*_TITLE, *_BODY) route through env: + toJSON(), the job-level if correctly filters to closed PRs and excludes bot comments (so the bot's own redirect reply won't recurse), and the permission-lookup catch defaults to "non-maintainer" which is the right safe default.

Looks good to merge.