Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions actions/setup/js/resolve_mentions_from_payload.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ async function resolveAllowedMentionsFromPayload(context, github, core, mentions
}

// Get configuration options (with defaults)
const allowTeamMembers = mentionsConfig?.allowTeamMembers !== false; // default: true
const allowCollaboratorMentions = (mentionsConfig?.allowedCollaborators ?? mentionsConfig?.allowTeamMembers) !== false; // default: true
const allowContext = mentionsConfig?.allowContext !== false; // default: true
const allowedList = mentionsConfig?.allowed || [];
const allowedTeams = mentionsConfig?.allowedTeams || [];
Expand All @@ -202,7 +202,7 @@ async function resolveAllowedMentionsFromPayload(context, github, core, mentions
knownAuthors.push(...allowedList.filter(alias => typeof alias === "string" && alias.length > 0));
}

// Add members from allowed-teams (always included regardless of allow-team-members setting)
// Add members from allowed-teams (always included regardless of collaborator mention setting)
if (Array.isArray(allowedTeams) && allowedTeams.length > 0) {
core.info(`[MENTIONS] Fetching members for ${allowedTeams.length} configured team(s)`);
for (const teamEntry of allowedTeams) {
Expand Down Expand Up @@ -231,9 +231,9 @@ async function resolveAllowedMentionsFromPayload(context, github, core, mentions
deduplicatedKnownAuthors.push(author);
}

// If allow-team-members is disabled, only use known authors (context + allowed list)
if (!allowTeamMembers) {
core.info(`[MENTIONS] Team members disabled - only allowing context (${deduplicatedKnownAuthors.length} users)`);
// If collaborator mentions are disabled, only use known authors (context + allowed list)
if (!allowCollaboratorMentions) {
core.info(`[MENTIONS] Collaborator mentions disabled - only allowing context (${deduplicatedKnownAuthors.length} users)`);
if (deduplicatedKnownAuthors.length > maxMentions) {
core.warning(`[MENTIONS] Mention limit exceeded: ${deduplicatedKnownAuthors.length} mentions, limiting to ${maxMentions}`);
}
Expand Down
14 changes: 14 additions & 0 deletions actions/setup/js/resolve_mentions_from_payload.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,20 @@ describe("resolveAllowedMentionsFromPayload", () => {
expect(result).not.toContain("alice");
});

it("respects allowedCollaborators: false", async () => {
const context = {
eventName: "issues",
payload: { issue: { user: { login: "alice", type: "User" }, assignees: [] } },
repo: { owner: "o", repo: "r" },
};
const result = await resolveAllowedMentionsFromPayload(context, mockGithub, mockCore, {
allowContext: false,
allowedCollaborators: false,
allowed: ["trusted-user"],
});
expect(result).toEqual(["trusted-user"]);
});

it("resolves mentions with team members enabled", async () => {
const context = {
eventName: "issues",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# ADR-40394: Rename `allow-team-members` to `allowed-collaborators` in `safe-outputs.mentions`

**Date**: 2026-06-19
**Status**: Draft

## Context

The `safe-outputs.mentions` configuration historically exposed a boolean field named `allow-team-members` to control whether repository collaborators could be @mentioned. The rest of the mentions configuration uses an `allowed-*` naming family (`allowed`, `allowed-teams`), so `allow-team-members` was inconsistent with the surrounding vocabulary and described its subject ("collaborators with any permission level") imprecisely. Because workflow frontmatter is authored by users in repositories outside this codebase, any rename must avoid silently breaking existing workflows that still set the old key. This decision concerns how to evolve a public configuration field name without a breaking change.

## Decision

We will rename the field to `allowed-collaborators` as the canonical name and retain `allow-team-members` as a backward-compatible deprecated alias. Concretely: `MentionsConfig.AllowTeamMembers` becomes `AllowedCollaborators` (YAML tag `allowed-collaborators`, runtime JSON key `allowedCollaborators`); `parseMentionsConfig` reads `allowed-collaborators` first and falls back to `allow-team-members`; the JSON schema marks the old key `deprecated` with an `x-deprecation-message`; and a `gh aw fix` codemod (`mentions-allow-team-members-to-allowed-collaborators`) rewrites the old key to the new one in-place while preserving indentation, inline comments, and the markdown body. The codemod is a no-op when the key is absent, when `mentions` is a bare boolean, or when the field is already migrated.

## Alternatives Considered

### Alternative 1: Hard rename with no alias
Remove `allow-team-members` entirely and accept only `allowed-collaborators`. Rejected because it would immediately break every existing workflow that still sets the old key, with no migration path, violating the non-negotiable backward-compatibility constraint.

### Alternative 2: Leave the field named `allow-team-members`
Keep the existing name to avoid any migration work. Rejected because it perpetuates the naming inconsistency with the `allowed-*` family and the imprecise "team-members" terminology, and the cost of a backward-compatible rename plus an automated codemod is low.

## Consequences

### Positive
- The field name is consistent with the `allowed-*` configuration family and more accurately describes repository collaborators.
- Existing workflows continue to compile unchanged via the deprecated-alias fallback, and `gh aw fix` migrates them automatically.

### Negative
- The parser and schema must carry two keys for the same setting until the deprecated alias is removed, increasing maintenance surface.
- The deprecated `allow-team-members` entry lingers in the schema and documentation, which can confuse new authors about which key to use.

### Neutral
- The runtime JSON key emitted to the handler changes from `allowTeamMembers` to `allowedCollaborators`; downstream consumers read the new key.
- A future ADR will be required to schedule and execute removal of the deprecated alias once adoption is confirmed.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27852098024) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
151 changes: 151 additions & 0 deletions pkg/cli/codemod_mentions_allow_team_members.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package cli

import (
"strings"

"github.com/github/gh-aw/pkg/logger"
)

var mentionsAllowTeamMembersCodemodLog = logger.New("cli:codemod_mentions_allow_team_members")

func getMentionsAllowTeamMembersCodemod() Codemod {
return Codemod{
ID: "mentions-allow-team-members-to-allowed-collaborators",
Name: "Rename allow-team-members to allowed-collaborators in mentions",
Description: "Renames allow-team-members to allowed-collaborators in safe-outputs.mentions.",
IntroducedIn: "1.0.0",
Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
if !mentionsAllowTeamMembersNeedsMigration(frontmatter) {
return content, false, nil
}

newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
return renameMentionsAllowTeamMembers(lines)
})
if applied {
mentionsAllowTeamMembersCodemodLog.Print("Renamed allow-team-members to allowed-collaborators in safe-outputs.mentions")
}
return newContent, applied, err
},
}
}

func mentionsAllowTeamMembersNeedsMigration(frontmatter map[string]any) bool {
safeOutputsAny, ok := frontmatter["safe-outputs"]
if !ok {
return false
}
safeOutputsMap, ok := safeOutputsAny.(map[string]any)
if !ok {
return false
}
mentionsAny, ok := safeOutputsMap["mentions"]
if !ok {
return false
}
mentionsMap, ok := mentionsAny.(map[string]any)
if !ok {
return false
}

_, hasOld := mentionsMap["allow-team-members"]
_, hasNew := mentionsMap["allowed-collaborators"]
return hasOld && !hasNew

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.

[/tdd] When both allow-team-members and allowed-collaborators are present simultaneously, mentionsAllowTeamMembersNeedsMigration returns false and the codemod silently skips. The old deprecated key stays in the file even after the user runs gh aw fix.

The TestMentionsAllowTeamMembersCodemod_NoOp_AlreadyMigrated test only covers the case where allow-team-members is absent — not the "both keys present" case. A user who hand-edited their YAML to add allowed-collaborators but forgot to remove the old key would be surprised that gh aw fix does nothing.

💡 Suggested additional test
func TestMentionsAllowTeamMembersCodemod_NoOp_BothKeysPresent(t *testing.T) {
    codemod := getMentionsAllowTeamMembersCodemod()
    content := `---
safe-outputs:
  mentions:
    allow-team-members: false
    allowed-collaborators: false
---`
    frontmatter := map[string]any{
        "safe-outputs": map[string]any{
            "mentions": map[string]any{
                "allow-team-members":    false,
                "allowed-collaborators": false,
            },
        },
    }
    result, applied, err := codemod.Apply(content, frontmatter)
    require.NoError(t, err)
    assert.False(t, applied)
    assert.Equal(t, content, result)
}

This documents the intended no-op and prevents a future refactor from accidentally "fixing" both-key documents in unexpected ways.

}

func renameMentionsAllowTeamMembers(lines []string) ([]string, bool) {
result := make([]string, 0, len(lines))
modified := false

inSafeOutputs := false
safeOutputsIndent := ""
safeOutputsChildIndent := ""
inMentions := false
mentionsIndent := ""
mentionsChildIndent := ""

for i, line := range lines {
trimmed := strings.TrimSpace(line)
indent := getIndentation(line)

if !strings.HasPrefix(trimmed, "#") {
if inSafeOutputs && hasExitedBlock(line, safeOutputsIndent) {
inSafeOutputs = false
safeOutputsChildIndent = ""
inMentions = false
mentionsIndent = ""
mentionsChildIndent = ""
}
if inMentions && hasExitedBlock(line, mentionsIndent) {
inMentions = false
mentionsIndent = ""
mentionsChildIndent = ""
}
}

if strings.HasPrefix(trimmed, "safe-outputs:") {
inSafeOutputs = true
safeOutputsIndent = indent
safeOutputsChildIndent = ""
inMentions = false
mentionsIndent = ""
mentionsChildIndent = ""
result = append(result, line)
continue
}

if inSafeOutputs && isDescendant(indent, safeOutputsIndent) && !strings.HasPrefix(trimmed, "#") {
if (safeOutputsChildIndent == "" || indent == safeOutputsChildIndent) && strings.HasPrefix(trimmed, "mentions:") {
if safeOutputsChildIndent == "" {
safeOutputsChildIndent = indent
}
if strings.HasSuffix(trimmed, ":") {
inMentions = true
mentionsIndent = indent
mentionsChildIndent = ""
} else {
inMentions = false
mentionsIndent = ""
mentionsChildIndent = ""
if strings.Contains(trimmed, "allow-team-members:") {
newLine := strings.Replace(line, "allow-team-members:", "allowed-collaborators:", 1)
result = append(result, newLine)
modified = true
mentionsAllowTeamMembersCodemodLog.Printf("Renamed allow-team-members to allowed-collaborators in safe-outputs.mentions on line %d", i+1)
continue
}
}
result = append(result, line)
continue
}
if strings.HasSuffix(trimmed, ":") && (safeOutputsChildIndent == "" || indent == safeOutputsChildIndent) {
if safeOutputsChildIndent == "" {
safeOutputsChildIndent = indent
}
inMentions = false
mentionsIndent = ""
mentionsChildIndent = ""
result = append(result, line)
continue
}
}

if inMentions && mentionsChildIndent == "" && isDescendant(indent, mentionsIndent) && trimmed != "" && !strings.HasPrefix(trimmed, "#") {
mentionsChildIndent = indent
}

if inMentions && indent == mentionsChildIndent && strings.HasPrefix(trimmed, "allow-team-members:") {
newLine, replaced := findAndReplaceInLine(line, "allow-team-members", "allowed-collaborators")
if replaced {
result = append(result, newLine)
modified = true
mentionsAllowTeamMembersCodemodLog.Printf("Renamed allow-team-members to allowed-collaborators in safe-outputs.mentions on line %d", i+1)
continue
}
}

result = append(result, line)
}

return result, modified
}
Loading
Loading