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
5 changes: 5 additions & 0 deletions .changeset/patch-allow-wildcard-target-repo-safe-outputs.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions actions/setup/js/add_comment.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,38 @@ describe("add_comment", () => {
expect(result.itemNumber).toBe(42);
expect(result.isDiscussion).toBe(false);
});

it('should allow commenting on any repo when target-repo is "*"', async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let capturedOwner = null;
let capturedRepo = null;
mockGithub.rest.issues.createComment = async params => {
capturedOwner = params.owner;
capturedRepo = params.repo;
return {
data: {
id: 12345,
html_url: `https://github.com/${params.owner}/${params.repo}/issues/${params.issue_number}#issuecomment-12345`,
},
};
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ "target-repo": "*", target: "triggering" }); })()`);

const message = {
type: "add_comment",
item_number: 5,
repo: "other-org/other-repo",
body: "Cross-repo comment",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(capturedOwner).toBe("other-org");
expect(capturedRepo).toBe("other-repo");
});
});

describe("discussion support", () => {
Expand Down
42 changes: 42 additions & 0 deletions actions/setup/js/close_issue.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,48 @@ describe("close_issue", () => {
expect(updateCalls[0].repo).toBe("gh-aw");
});

it("should close issue in any repo when target-repo is wildcard *", async () => {
const handler = await main({
max: 10,
"target-repo": "*",
});
const updateCalls = [];

mockGithub.rest.issues.get = async params => ({
data: {
number: params.issue_number,
title: "Test Issue",
labels: [],
html_url: `https://github.com/${params.owner}/${params.repo}/issues/${params.issue_number}`,
state: "open",
},
});

mockGithub.rest.issues.update = async params => {
updateCalls.push(params);
return {
data: {
number: params.issue_number,
title: "Test Issue",
html_url: `https://github.com/${params.owner}/${params.repo}/issues/${params.issue_number}`,
},
};
};

const result = await handler(
{
issue_number: 200,
repo: "any-org/any-repo",
body: "Closing",
},
{}
);

expect(result.success).toBe(true);
expect(updateCalls[0].owner).toBe("any-org");
expect(updateCalls[0].repo).toBe("any-repo");
});

it("should use default state_reason 'COMPLETED' when not specified", async () => {
const handler = await main({ max: 10 });
const updateCalls = [];
Expand Down
20 changes: 20 additions & 0 deletions actions/setup/js/create_discussion_labels.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -433,4 +433,24 @@ describe("create_discussion with labels", () => {
expect(result.error).toContain("Cannot add more than 10 labels");
expect(result.error).toContain("received 11");
});

it('should create discussion in any repo when target-repo is "*"', async () => {
const config = {
"target-repo": "*",
category: "general",
};

const handler = await createDiscussionMain(config);

const message = {
title: "Cross-repo Discussion",
body: "Discussion body",
repo: "other-org/other-repo",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.number).toBe(42);
});
});
28 changes: 28 additions & 0 deletions actions/setup/js/create_issue.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,34 @@ describe("create_issue", () => {
})
);
});
it("should create issue in specified repo when target-repo is wildcard *", async () => {
const handler = await main({
"target-repo": "*",
});
await handler({
title: "Test",
repo: "any-org/any-repo",
});

expect(mockGithub.rest.issues.create).toHaveBeenCalledWith(
expect.objectContaining({
owner: "any-org",
repo: "any-repo",
})
);
});

it("should reject invalid repo slug when target-repo is wildcard *", async () => {
const handler = await main({
"target-repo": "*",
});
const result = await handler({
title: "Test",
repo: "bare-repo-without-slash",
});

expect(result.success).toBe(false);
});
});

describe("temporary ID management", () => {
Expand Down
111 changes: 111 additions & 0 deletions actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -893,3 +893,114 @@ describe("create_pull_request - configured reviewers", () => {
expect(global.github.rest.pulls.requestReviewers).toHaveBeenCalledWith(expect.objectContaining({ reviewers: ["user1", "user2"] }));
});
});

describe("create_pull_request - wildcard target-repo", () => {
let tempDir;
let originalEnv;

beforeEach(() => {
originalEnv = { ...process.env };
process.env.GH_AW_WORKFLOW_ID = "test-workflow";
process.env.GITHUB_REPOSITORY = "test-owner/test-repo";
process.env.GITHUB_BASE_REF = "main";
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-wildcard-test-"));

global.core = {
info: vi.fn(),
warning: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
setFailed: vi.fn(),
setOutput: vi.fn(),
startGroup: vi.fn(),
endGroup: vi.fn(),
summary: {
addRaw: vi.fn().mockReturnThis(),
write: vi.fn().mockResolvedValue(undefined),
},
};
global.github = {
rest: {
pulls: {
create: vi.fn().mockResolvedValue({ data: { number: 99, html_url: "https://github.com/any-org/any-repo/pull/99", node_id: "PR_99" } }),
requestReviewers: vi.fn().mockResolvedValue({}),
},
repos: {
get: vi.fn().mockResolvedValue({ data: { default_branch: "main" } }),
},
issues: {
addLabels: vi.fn().mockResolvedValue({}),
},
},
graphql: vi.fn(),
};
global.context = {
eventName: "workflow_dispatch",
repo: { owner: "test-owner", repo: "test-repo" },
payload: {},
};
global.exec = {
exec: vi.fn().mockResolvedValue(0),
getExecOutput: vi.fn().mockResolvedValue({ exitCode: 0, stdout: "", stderr: "" }),
};

delete require.cache[require.resolve("./create_pull_request.cjs")];
});

afterEach(() => {
for (const key of Object.keys(process.env)) {
if (!(key in originalEnv)) {
delete process.env[key];
}
}
Object.assign(process.env, originalEnv);

if (tempDir && fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true, force: true });
}

delete global.core;
delete global.github;
delete global.context;
delete global.exec;
vi.clearAllMocks();
});

it('should create PR in any repo when target-repo is "*"', async () => {
const { main } = require("./create_pull_request.cjs");
const handler = await main({ "target-repo": "*", allow_empty: true });

const result = await handler(
{
title: "Test PR",
body: "Test body",
repo: "any-org/any-repo",
},
{}
);

expect(result.success).toBe(true);
expect(global.github.rest.pulls.create).toHaveBeenCalledWith(
expect.objectContaining({
owner: "any-org",
repo: "any-repo",
})
);
});

it('should reject invalid repo slug when target-repo is "*"', async () => {
const { main } = require("./create_pull_request.cjs");
const handler = await main({ "target-repo": "*", allow_empty: true });

const result = await handler(
{
title: "Test PR",
body: "Test body",
repo: "not-a-valid-slug",
},
{}
);

expect(result.success).toBe(false);
});
});
40 changes: 40 additions & 0 deletions actions/setup/js/repo_helpers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -506,5 +506,45 @@ describe("repo_helpers", () => {
expect(result.allowedRepos.has("org/repo-2")).toBe(true);
expect(result.allowedRepos.has("org/repo-3")).toBe(true);
});

it('should pass through wildcard "*" as defaultTargetRepo', async () => {
const { resolveTargetRepoConfig } = await import("./repo_helpers.cjs");
const config = { "target-repo": "*" };

const result = resolveTargetRepoConfig(config);

expect(result.defaultTargetRepo).toBe("*");
expect(result.allowedRepos.size).toBe(0);
});
});

describe('resolveAndValidateRepo with wildcard target-repo "*"', () => {
it("should allow any valid owner/repo slug when defaultTargetRepo is *", async () => {
const { resolveAndValidateRepo } = await import("./repo_helpers.cjs");
const result = resolveAndValidateRepo({ repo: "any-org/any-repo" }, "*", new Set(), "test");

expect(result.success).toBe(true);
expect(result.repo).toBe("any-org/any-repo");
expect(result.repoParts).toEqual({ owner: "any-org", repo: "any-repo" });
});

it("should use item.repo when defaultTargetRepo is * and no repo in item", async () => {
const { resolveAndValidateRepo } = await import("./repo_helpers.cjs");
// When item has no repo and defaultTargetRepo is "*", itemRepo becomes "*"
// which fails parseRepoSlug — so the caller should always supply item.repo with *
const result = resolveAndValidateRepo({}, "*", new Set(), "test");

// "*" is not a valid owner/repo slug (no slash with two parts)
expect(result.success).toBe(false);
});

it("should reject bare repo name (no slash) when defaultTargetRepo is *", async () => {
const { resolveAndValidateRepo } = await import("./repo_helpers.cjs");
const result = resolveAndValidateRepo({ repo: "bare-repo" }, "*", new Set(), "test");

// bare-repo qualifies using defaultRepo's org, but "*" has no org → falls back to bare name
// which is invalid as owner/repo
expect(result.success).toBe(false);
});
});
});
5 changes: 0 additions & 5 deletions pkg/workflow/add_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
config.Max = defaultIntStr(1)
}

// Validate target-repo (wildcard "*" is not allowed)
if validateTargetRepoSlug(config.TargetRepoSlug, addCommentLog) {
return nil // Invalid configuration, return nil to cause validation error
}

// Validate discussion field - must be true if present
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the target-repo: "*" wildcard is allowed, the handler config correctly receives and passes the wildcard value. The existing discussion field validation below still works correctly. ✅

if config.Discussion != nil && !*config.Discussion {
addCommentLog.Print("Invalid discussion: must be true if present")
Expand Down
16 changes: 16 additions & 0 deletions pkg/workflow/add_comment_target_repo_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ func TestAddCommentTargetRepoIntegration(t *testing.T) {
shouldHaveTargetRepo: false, // Trial mode sets env var, not config
expectedTargetRepoValue: "", // Not checked
},
{
name: "target-repo wildcard should be in handler config",
frontmatter: map[string]any{
"name": "Test Workflow",
"engine": "copilot",
"safe-outputs": map[string]any{
"add-comment": map[string]any{
"max": 1,
"target": "*",
"target-repo": "*",
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Good addition of a test case for the wildcard target-repo: "*" scenario. This ensures the change is properly validated at the integration level. Consider also testing with an actual mock comment to verify the end-to-end flow works with the wildcard target.

},
shouldHaveTargetRepo: true,
expectedTargetRepoValue: "*",
},
{
name: "no target-repo and no trial should not have target-repo in handler config",
frontmatter: map[string]any{
Expand Down
10 changes: 6 additions & 4 deletions pkg/workflow/add_comment_target_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,17 @@ func TestAddCommentsConfigTargetRepo(t *testing.T) {
shouldBeNil: false,
},
{
name: "target-repo with wildcard should be rejected",
name: "target-repo with wildcard is allowed",
configMap: map[string]any{
"add-comment": map[string]any{
"max": 5,
"target": "123",
"max": 1,
"target": "*",
"target-repo": "*",
},
},
shouldBeNil: true, // Configuration should be nil due to validation
expectedTarget: "*",
expectedRepo: "*",
shouldBeNil: false, // Wildcard "*" is a valid target-repo for add-comment
},
{
name: "target-repo without target field",
Copy link
Contributor

Choose a reason for hiding this comment

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

Good change — the test name update from "should be rejected" to "is allowed" correctly reflects the new intended behavior that wildcard * is a valid target-repo value for add-comment safe-outputs.

Expand Down
5 changes: 0 additions & 5 deletions pkg/workflow/add_reviewer.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ func (c *Compiler) parseAddReviewerConfig(outputMap map[string]any) *AddReviewer
config.Max = defaultIntStr(3)
}

// Validate target-repo (wildcard "*" is not allowed for safe outputs)
if validateTargetRepoSlug(config.TargetRepoSlug, addReviewerLog) {
return nil
}

addReviewerLog.Printf("Parsed add-reviewer config: allowed_reviewers=%d, target=%s", len(config.Reviewers), config.Target)

return &config
Expand Down
6 changes: 0 additions & 6 deletions pkg/workflow/close_entity_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,6 @@ func (c *Compiler) parseCloseEntityConfig(outputMap map[string]any, params Close
logger.Printf("Set default max to 1 for %s", params.ConfigKey)
}

// Validate target-repo (wildcard "*" is not allowed)
if validateTargetRepoSlug(config.TargetRepoSlug, logger) {
logger.Printf("Invalid target-repo slug for %s", params.ConfigKey)
return nil
}

logger.Printf("Parsed %s configuration: max=%s, target=%s", params.ConfigKey, *config.Max, config.Target)

return &config
Expand Down
Loading