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
30 changes: 25 additions & 5 deletions actions/setup/js/repo_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,16 @@ function getDefaultTargetRepo(config) {
if (targetRepoSlug) {
return targetRepoSlug;
}
// Fall back to context repo
return `${context.repo.owner}/${context.repo.repo}`;
// Fall back to context repo (only available in github-script or shim-provided context)
if (typeof context !== "undefined" && context.repo?.owner && context.repo?.repo) {
return `${context.repo.owner}/${context.repo.repo}`;
}
// Fall back to GITHUB_REPOSITORY env var (available in standalone daemon mode)
const githubRepo = process.env.GITHUB_REPOSITORY;
if (githubRepo) {
return githubRepo;
}
return "";
}

/**
Expand Down Expand Up @@ -214,11 +222,23 @@ function resolveTargetRepoConfig(config) {
* @returns {RepoResolutionResult}
*/
function resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, operationType) {
// Determine target repository for this operation
const itemRepo = item.repo ? String(item.repo).trim() : defaultTargetRepo;
// Normalize the default target repo (may be empty if not configured)
const trimmedDefaultTargetRepo = defaultTargetRepo ? String(defaultTargetRepo).trim() : "";

// Determine target repository for this operation, allowing item.repo to override
const rawItemRepo = item && item.repo != null ? String(item.repo).trim() : "";
const itemRepo = rawItemRepo || trimmedDefaultTargetRepo;

// If we still don't have a repo after considering overrides, treat as configuration/environment issue
if (!itemRepo) {
return {
success: false,
error: `Unable to determine target repository for ${operationType}. Set GH_AW_TARGET_REPO_SLUG, ensure GITHUB_REPOSITORY is available, or configure target-repo in safe-outputs settings.`,
};
}
Comment on lines 224 to 238
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

The new !defaultTargetRepo guard returns an error even when item.repo is explicitly provided. This is a behavior regression for tools/messages that support a repo override (e.g., unassign_from_user in safe_outputs_tools.json), where operations should still be able to proceed (and be validated against allowedRepos) even if the default repo can’t be inferred. Consider only erroring when both the resolved repo (after trimming) is empty and there’s no item.repo, or moving repo selection (itemRepo) before the guard and checking that instead.

This issue also appears on line 112 of the same file.

See below for a potential fix:

  // Normalize and trim the default target repo (may be empty if not configured)
  const trimmedDefaultTargetRepo = defaultTargetRepo ? String(defaultTargetRepo).trim() : "";

  // Determine target repository for this operation, allowing item.repo to override
  const rawItemRepo = item && item.repo != null ? String(item.repo).trim() : "";
  const itemRepo = rawItemRepo || trimmedDefaultTargetRepo;

  // If we still don't have a repo after considering overrides, treat as configuration/environment issue
  if (!itemRepo) {
    return {
      success: false,
      error: `Unable to determine target repository for ${operationType}. Set GH_AW_TARGET_REPO_SLUG, ensure GITHUB_REPOSITORY is available, or configure target-repo in safe-outputs settings.`,
    };
  }

  // Validate the repository is allowed
  const repoValidation = validateRepo(itemRepo, trimmedDefaultTargetRepo, allowedRepos);

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 952f6cb. resolveAndValidateRepo now extracts item.repo first and only errors when both item.repo and defaultTargetRepo are empty, following the suggested pattern. I also updated validateRepo to receive trimmedDefaultTargetRepo and added a test for the item.repo-with-empty-default case.


// Validate the repository is allowed
const repoValidation = validateRepo(itemRepo, defaultTargetRepo, allowedRepos);
const repoValidation = validateRepo(itemRepo, trimmedDefaultTargetRepo, allowedRepos);
if (!repoValidation.valid) {
// When valid is false, error is guaranteed to be non-null
const errorMessage = repoValidation.error;
Expand Down
51 changes: 51 additions & 0 deletions actions/setup/js/repo_helpers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe("repo_helpers", () => {
beforeEach(() => {
vi.resetModules();
delete process.env.GH_AW_TARGET_REPO_SLUG;
delete process.env.GITHUB_REPOSITORY;
global.context = mockContext;
});

Expand Down Expand Up @@ -97,6 +98,31 @@ describe("repo_helpers", () => {
const result = getDefaultTargetRepo();
expect(result).toBe("test-owner/test-repo");
});

it("should use GITHUB_REPOSITORY env var when context is not defined", async () => {
process.env.GITHUB_REPOSITORY = "env-owner/env-repo";
// @ts-expect-error - Simulating standalone daemon where context is not available
delete global.context;
const { getDefaultTargetRepo } = await import("./repo_helpers.cjs");
const result = getDefaultTargetRepo();
expect(result).toBe("env-owner/env-repo");
});

it("should prefer GH_AW_TARGET_REPO_SLUG over GITHUB_REPOSITORY", async () => {
process.env.GH_AW_TARGET_REPO_SLUG = "slug-org/slug-repo";
process.env.GITHUB_REPOSITORY = "env-owner/env-repo";
const { getDefaultTargetRepo } = await import("./repo_helpers.cjs");
const result = getDefaultTargetRepo();
expect(result).toBe("slug-org/slug-repo");
});

it("should return empty string when no config, no env vars, and no context", async () => {
// @ts-expect-error - Simulating standalone daemon where context is not available
delete global.context;
const { getDefaultTargetRepo } = await import("./repo_helpers.cjs");
const result = getDefaultTargetRepo();
expect(result).toBe("");
});
});

describe("isRepoAllowed", () => {
Expand Down Expand Up @@ -371,6 +397,31 @@ describe("repo_helpers", () => {
expect(result.repo).toBe("github/gh-aw");
expect(result.repoParts).toEqual({ owner: "github", repo: "gh-aw" });
});

it("should fail with descriptive error when defaultTargetRepo is empty", async () => {
const { resolveAndValidateRepo } = await import("./repo_helpers.cjs");
const item = {};
const allowedRepos = new Set();

const result = resolveAndValidateRepo(item, "", allowedRepos, "pull request");

expect(result.success).toBe(false);
expect(result.error).toContain("Unable to determine target repository for pull request");
expect(result.error).toContain("GH_AW_TARGET_REPO_SLUG");
expect(result.error).toContain("GITHUB_REPOSITORY");
});

it("should succeed when item.repo is provided even if defaultTargetRepo is empty", async () => {
const { resolveAndValidateRepo } = await import("./repo_helpers.cjs");
const item = { repo: "org/explicit-repo" };
const allowedRepos = new Set(["org/explicit-repo"]);

const result = resolveAndValidateRepo(item, "", allowedRepos, "pull request");

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

describe("resolveTargetRepoConfig", () => {
Expand Down
2 changes: 2 additions & 0 deletions actions/setup/js/unassign_from_user.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ describe("unassign_from_user (Handler Factory Architecture)", () => {

beforeEach(async () => {
vi.clearAllMocks();
delete process.env.GITHUB_REPOSITORY;
delete process.env.GH_AW_TARGET_REPO_SLUG;

const { main } = require("./unassign_from_user.cjs");
handler = await main({
Expand Down
Loading