Skip to content

fix: safe output handlers now respect target-repo config#35901

Merged
dsyme merged 7 commits into
mainfrom
fix/safe-output-handlers-respect-target-repo
May 30, 2026
Merged

fix: safe output handlers now respect target-repo config#35901
dsyme merged 7 commits into
mainfrom
fix/safe-output-handlers-respect-target-repo

Conversation

@dsyme
Copy link
Copy Markdown
Collaborator

@dsyme dsyme commented May 30, 2026

Cross-repo support for safe-output handlers + bug fixes

Summary

This PR delivers two main themes:

  1. Cross-repo support for safe-output action handlers — five JS action handlers (assign_milestone, close_discussion, link_sub_issue, mark_pull_request_as_ready_for_review, and close_pull_request) now resolve their target repository via resolveTargetRepoConfig / resolveAndValidateRepo from repo_helpers.cjs instead of blindly reading context.repo. The Go-side tool_description_enhancer was updated in parallel to inject TargetRepoSlug constraints and max-count descriptions for these tools.

  2. Reliability / CI bug fixes — a mutex defer correctness fix in remote_fetch.go, a missing directory guard in copilot_agents.go, two test isolation fixes (missing mocks, missing skip condition), and a pull-request-finding fix.


Changed files

Actions — cross-repo handler updates (actions/setup/js/)

File Change
assign_milestone.cjs Added resolveTargetRepoConfig + resolveAndValidateRepo for cross-repo milestone assignment
assign_milestone.test.cjs New target-repo support suite: default fallback, explicit config, allowed list, rejection
close_discussion.cjs Imported repo helpers; cross-repo support wired in
link_sub_issue.cjs Replaced context.repo with configurable repo resolution
link_sub_issue.test.cjs Tests for config-driven repo, context fallback, and allowed_repos rejection
mark_pull_request_as_ready_for_review.cjs All context.repo owner/repo references replaced with resolved values
mark_pull_request_as_ready_for_review.test.cjs Four new tests: config-driven, fallback, allowed-list accept, reject

Go — tool description enhancer (pkg/workflow/)

File Change
tool_description_enhancer.go TargetRepoSlug constraints + max-count for mark_pull_request_as_ready_for_review, close_discussion, close_pr, link_sub_issue, assign_milestone
tool_description_enhancer_test.go Matching tests for all new constraints and max-count
schemas/awf-config.schema.json Whitespace-only: single-element enum arrays reformatted to one line

Go — bug fixes (pkg/)

File Change
pkg/parser/remote_fetch.go Mutex defer refactored to use IIFEs in getOrCreateListRepoClone; eliminates manual unlock risk
pkg/cli/copilot_agents.go Added os.IsNotExist guard for absent .github/aw directory; returns safe default instead of propagating error
pkg/cli/logs_json_stderr_order_test.go Added skip for "failed to list workflow runs" to prevent false CI failures
pkg/cli/update_manifest_test.go Stubbed listPackageDirSubdirsForHost / listPackageDirFilesForHost to block real network calls
pkg/cli/add_package_manifest_test.go Added skills/my-skill/SKILL.md stub to cover skill manifest path

Impact

  • No breaking changes.
  • Cross-repo handler changes are high-impact functional additions: workflows with a target-repo config will now correctly fan out actions to the configured repo instead of always operating on the triggering repo.
  • Mutex and directory-guard fixes prevent potential data races and panics under concurrent or fresh-install conditions.
  • Test fixes improve CI reliability in environments without full workflow-run access.

Testing

  • New target-repo support test suites added for all three modified JS handlers.
  • tool_description_enhancer_test.go covers all new constraint cases.
  • Existing tests strengthened with proper mocks and skip conditions.

Generated by PR Description Updater for issue #35901 · sonnet46 2.1M ·

Several safe output JS handlers were hardcoding context.repo.owner/repo
instead of reading from the target-repo config field. This caused
cross-repository routing to be silently ignored when operators configured
a target-repo constraint.

Fixed handlers:
- close_pull_request.cjs
- mark_pull_request_as_ready_for_review.cjs
- close_discussion.cjs
- link_sub_issue.cjs
- assign_milestone.cjs

Each handler now calls resolveTargetRepoConfig(config) to read the
target-repo and allowed_repos config fields, and resolveAndValidateRepo
to validate individual message targets against them.

Also updated tool_description_enhancer.go to emit TargetRepoSlug
constraints in tool descriptions shown to AI agents for all five handlers,
ensuring agents are aware of the configured repository restriction.

Added JS tests for all fixed handlers and Go tests for the tool
description enhancer changes.
Copilot AI review requested due to automatic review settings May 30, 2026 10:15
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes safe-output JS handlers that previously hardcoded context.repo so they now honor the target-repo (and where applicable allowed_repos) handler configuration, and surfaces the configured target repo in the tool descriptions shown to the agent.

Changes:

  • Five JS handlers (close_pull_request, mark_pull_request_as_ready_for_review, close_discussion, link_sub_issue, assign_milestone) now resolve the target repo from config via resolveTargetRepoConfig / resolveAndValidateRepo and use the resolved owner/repo for all GitHub API calls.
  • tool_description_enhancer.go appends TargetRepoSlug constraint text for those five tools (and adds a max-count line for mark_pull_request_as_ready_for_review).
  • New JS handler tests for target-repo routing and Go tests for the description enhancer; unrelated formatting compaction in awf-config.schema.json.
Show a summary per file
File Description
actions/setup/js/close_pull_request.cjs Resolve+validate per-item repo in resolveTarget and propagate owner/repo.
actions/setup/js/mark_pull_request_as_ready_for_review.cjs Resolve per-item repo and use it for filter, PR details, and comment calls.
actions/setup/js/close_discussion.cjs Manually parse defaultTargetRepo and use it for discussion lookup.
actions/setup/js/link_sub_issue.cjs Manually parse defaultTargetRepo and use it for issue resolution defaults.
actions/setup/js/assign_milestone.cjs Plumb resolved repo into milestone find/create/get/update calls; thread owner/repo through findMilestoneByTitle.
actions/setup/js/*.test.cjs (4 files) Add target-repo routing, fallback, allowed-repos, and rejection tests.
pkg/workflow/tool_description_enhancer.go Emit target-repo constraint strings and a max-count line for the five handlers.
pkg/workflow/tool_description_enhancer_test.go Verify the new constraints appear in tool descriptions.
pkg/workflow/schemas/awf-config.schema.json Pure formatting: collapse short enum/required arrays to one line.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 12/12 changed files
  • Comments generated: 4

Comment thread actions/setup/js/mark_pull_request_as_ready_for_review.cjs Outdated
Comment thread actions/setup/js/assign_milestone.cjs Outdated
Comment thread actions/setup/js/close_discussion.cjs Outdated
Comment thread actions/setup/js/link_sub_issue.cjs Outdated
dsyme and others added 2 commits May 30, 2026 11:20
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

…ndValidateRepo

- Use allowedRepos.size (not .length) and Array.from(allowedRepos).join(...)
  in mark_pull_request_as_ready_for_review.cjs and assign_milestone.cjs
  (allowedRepos is a Set<string> returned by resolveTargetRepoConfig)

- Add per-item resolveAndValidateRepo call to close_discussion.cjs so that
  item.repo overrides and allowed_repos validation apply per message

- Add per-item resolveAndValidateRepo call to link_sub_issue.cjs and pass
  the resolved owner/repo to resolveRepoIssueTarget for both parent and
  sub-issue; ignored allowed_repos now validated

- Add JS tests covering allowed_repos rejection and item.repo override for
  close_discussion and link_sub_issue
@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown
Contributor

@copilot review all comments and address unresolved review feedback.

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1M ·

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

… logs order tests

TestLogsJSONOutputBeforeStderr and TestLogsJSONAndStderrRedirected were
failing when gh CLI is available but the test workflow name doesn't exist,
returning 'failed to list workflow runs (exit code 1)' which wasn't in the
skip condition list. Added this error string to both skip checks.
@dsyme dsyme merged commit 715d8a2 into main May 30, 2026
26 checks passed
@dsyme dsyme deleted the fix/safe-output-handlers-respect-target-repo branch May 30, 2026 12:15
@github-actions
Copy link
Copy Markdown
Contributor

✅ smoke-ci: safeoutputs CLI comment + comment-memory run (26683528903)

Generated by 🧪 Smoke CI for issue #35901 ·

github-actions Bot added a commit that referenced this pull request May 30, 2026
* fix: safe output handlers now respect target-repo config

Several safe output JS handlers were hardcoding context.repo.owner/repo
instead of reading from the target-repo config field. This caused
cross-repository routing to be silently ignored when operators configured
a target-repo constraint.

Fixed handlers:
- close_pull_request.cjs
- mark_pull_request_as_ready_for_review.cjs
- close_discussion.cjs
- link_sub_issue.cjs
- assign_milestone.cjs

Each handler now calls resolveTargetRepoConfig(config) to read the
target-repo and allowed_repos config fields, and resolveAndValidateRepo
to validate individual message targets against them.

Also updated tool_description_enhancer.go to emit TargetRepoSlug
constraints in tool descriptions shown to AI agents for all five handlers,
ensuring agents are aware of the configured repository restriction.

Added JS tests for all fixed handlers and Go tests for the tool
description enhancer changes.

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: address PR review comments - Set API usage and per-item resolveAndValidateRepo

- Use allowedRepos.size (not .length) and Array.from(allowedRepos).join(...)
  in mark_pull_request_as_ready_for_review.cjs and assign_milestone.cjs
  (allowedRepos is a Set<string> returned by resolveTargetRepoConfig)

- Add per-item resolveAndValidateRepo call to close_discussion.cjs so that
  item.repo overrides and allowed_repos validation apply per message

- Add per-item resolveAndValidateRepo call to link_sub_issue.cjs and pass
  the resolved owner/repo to resolveRepoIssueTarget for both parent and
  sub-issue; ignored allowed_repos now validated

- Add JS tests covering allowed_repos rejection and item.repo override for
  close_discussion and link_sub_issue

* fix: resolve CGO CI failures - mutex defer, init skill dir, test mock

* fix: add missing mocks in TestUpdateManifestWorkflowGroup to prevent real network calls

* fix: add missing skip condition for 'failed to list workflow runs' in logs order tests

TestLogsJSONOutputBeforeStderr and TestLogsJSONAndStderrRedirected were
failing when gh CLI is available but the test workflow name doesn't exist,
returning 'failed to list workflow runs (exit code 1)' which wasn't in the
skip condition list. Added this error string to both skip checks.

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants