Allow aw.yml packages to install shared JavaScript modules by Copilot · Pull Request #60536 · github/gh-aw

Copilot · 2026-09-13T01:18:53Z

Package manifests rejected .mjs and .cjs resources targeting .github/workflows/shared/, preventing packages from shipping JavaScript helpers alongside shared workflows.

Manifest support
- Allow .mjs and .cjs resources under .github/workflows/shared/.
- Preserve path traversal and extension restrictions.
Package lifecycle
- Include shared scripts in package ownership, update, and removal handling.
Documentation
- Update the schema, package reference, specification, and changeset.

resources:
  - source: shared/runtime.mjs
    destination: .github/workflows/shared/runtime.mjs

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot

🟢 Approval recommended

The destination validation, ownership integration, tests, and documentation consistently implement the requested support.

Pull request overview

Adds package support for installing and managing shared .mjs and .cjs workflow helpers.

Changes:

Allows validated JavaScript module destinations under .github/workflows/shared/.
Integrates shared scripts with package ownership and lifecycle handling.
Adds tests, documentation, schema guidance, and a changeset.

File summaries

File	Description
`pkg/parser/schemas/aw_manifest_schema.json`	Documents supported shared scripts.
`pkg/cli/add_package_ownership.go`	Recognizes scripts as package resources.
`pkg/cli/add_package_manifest_resources.go`	Validates `.mjs` and `.cjs` destinations.
`pkg/cli/add_package_manifest_resources_test.go`	Tests allowed and rejected paths.
`docs/src/content/docs/specs/repository-package-manifest-specification.md`	Updates the package specification.
`docs/src/content/docs/reference/aw-yml-package-manifest.md`	Updates user-facing reference documentation.
`.changeset/patch-aw-yml-shared-js-resources.md`	Records the patch release change.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0
Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-13T01:34:47Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-09-13T01:34:51Z

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #60536

github-actions · 2026-09-13T01:35:06Z

⚠️ Security scanning failed for Design Decision Gate 🏗️. Review the logs for details.

No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-09-13T01:35:07Z

⚠️ PR Code Quality Reviewer failed during code quality review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-09-13T01:35:24Z

✅ Test Quality Sentinel completed test quality analysis.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🧪 Test quality analysis by Test Quality Sentinel

github-actions

Impeccable Review — refactor_cleanup

Applied distill + extract lens (backend validation/schema change extending an existing resource-allowlist pattern; no UI surface).

Findings: none blocking or high-signal. The change:

Reuses the existing validateManifestResourceDestination switch and isPackageResourceDestination predicate consistently across parse, ownership-tracking, and stale-removal paths (add_package_manifest_resources.go, add_package_ownership.go), keeping install/update/remove lifecycle uniform with the pre-existing .github/aw/ and ISSUE_TEMPLATE cases.
Case-insensitive .mjs/.cjs suffix check plus WorkflowsDirSlash+"shared/" prefix correctly excludes sibling dirs like shared-adjacent/ (verified via the new test file and go test ./pkg/cli/... -run TestValidateManifestResourceDestinationSharedWorkflowScripts, all passing).
Path traversal is still rejected upstream by cleanManifestRelativePath before this validator runs, so .github/workflows/shared/../runtime.mjs is blocked (covered by the added test).
Docs/spec/schema/changeset are all updated consistently with the new allowlist entry.

Ran go build ./... and the targeted test — both succeed.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 36.8 AIC · ⌖ 14.6 AIC · ⊞ 8.4K

github-actions

Skills-Based Review 🧠

Applied /tdd and /codebase-design to this small, well-scoped manifest-resources change. Two suggestions posted inline; nothing blocking.

📋 Key Themes & Highlights

Key Themes

Test coverage gap: unit tests cover the validation predicate well, but there's no end-to-end test exercising install + ownership-record round trip for the new .mjs/.cjs destination type, unlike the existing ISSUE_TEMPLATE coverage.
Duplicated matching logic: the shared/ prefix + extension check is implemented independently in both validateManifestResourceDestination and isSharedWorkflowScriptDestination, which could drift if either is updated later.

Positive Highlights

✅ Path traversal and extension restrictions are preserved and explicitly tested (including the ../ traversal negative case).
✅ Docs (reference, spec) and the changeset are updated consistently with the code change.
✅ Package ownership/ lifecycle wiring (isPackageResourceDestination) was updated alongside validation, keeping install/update/removal consistent.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 119.9 AIC · ⌖ 15 AIC · ⊞ 10.4K
_{Comment /matt to run again}

github-actions · 2026-09-13T01:48:12Z

+		"destination": ".github/workflows/shared/../runtime.mjs",
+	}, "aw.yml")
+	require.Error(t, err)
+}


[/tdd] Good unit coverage for validateManifestResourceDestination, but there's no end-to-end test proving a .mjs/.cjs resource actually gets written to disk and recorded in package ownership metadata (the existing TestAddWorkflowsWithTracking_PackageResourceWritesOwnershipRecord pattern in add_command_test.go only covers the ISSUE_TEMPLATE destination type).

💡 Suggested addition

Add a variant of TestAddWorkflowsWithTracking_PackageResourceWritesOwnershipRecord (or extend it via table-driven cases) using DestinationPath: ".github/workflows/shared/runtime.mjs", asserting the file is written verbatim and the ownership record's destination field matches. This closes the gap between "destination string is accepted" and "the new resource type round-trips through install + ownership + removal correctly," which is the actual behavior users depend on.

@copilot please address this.

Added TestAddWorkflowsWithTracking_PackageSharedScriptResourceWritesOwnershipRecord covering the .mjs destination end-to-end (write to disk + ownership record) in 7730687.

github-actions · 2026-09-13T01:48:12Z


 func validateManifestResourceDestination(destination string) error {
 	switch {
+	case strings.HasPrefix(destination, constants.WorkflowsDirSlash+"shared/"):


[/codebase-design] The new shared/ case duplicates the extension-matching logic already present in isSharedWorkflowScriptDestination (in add_package_ownership.go) with slightly different mechanics (strings.HasPrefix+TrimPrefix here vs. filepath.Clean+ToSlash there). Two independent implementations of "what counts as a shared JS resource" is a seam that will drift silently if one is updated later.

💡 Suggested consolidation

Extract a single helper, e.g. isSharedWorkflowScriptPath(destination string) (remaining string, ok bool), used by both validateManifestResourceDestination and isSharedWorkflowScriptDestination/isPackageResourceDestination. This keeps the acceptance rule (prefix + .mjs/.cjs suffix) defined in exactly one place, matching the deep-module principle of a single source of truth for a piece of domain logic reused across the manifest-parsing and ownership-tracking modules.

@copilot please address this.

validateManifestResourceDestination now delegates to isSharedWorkflowScriptDestination for the shared-script acceptance check, so the prefix+extension rule is defined in one place (7730687).

pelikhan · 2026-09-13T01:50:51Z

@copilot run pr-finisher skill

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-09-13T02:54:18Z

🎉 This pull request is included in a new release.

Release: v0.89.10

Support shared JavaScript package resources

6c0ff74

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI assigned Copilot and pelikhan Sep 13, 2026

Copilot created this pull request from a session on behalf of pelikhan September 13, 2026 01:19 View session

Copilot finished work on behalf of pelikhan September 13, 2026 01:19

Copilot AI requested a review from pelikhan September 13, 2026 01:19

pelikhan marked this pull request as ready for review September 13, 2026 01:21

Copilot AI balanced review requested due to automatic review settings September 13, 2026 01:21

Copilot started reviewing on behalf of pelikhan September 13, 2026 01:21 View session

Copilot AI reviewed Sep 13, 2026

View reviewed changes

github-actions Bot added the needs-review label Sep 13, 2026

github-actions Bot mentioned this pull request Sep 13, 2026

[aw] Test Quality Sentinel produced no safe outputs #60448

Closed

github-actions Bot approved these changes Sep 13, 2026

View reviewed changes

github-actions Bot reviewed Sep 13, 2026

View reviewed changes

Copilot started work on behalf of pelikhan September 13, 2026 01:51 View session

Consolidate shared script destination check and add e2e ownership test

7730687

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot finished work on behalf of pelikhan September 13, 2026 02:01

pelikhan merged commit 791ec67 into main Sep 13, 2026

pelikhan deleted the copilot/update-aw-yml-accept-mjs-cjs-files branch September 13, 2026 02:21

Conversation

Copilot AI commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Impeccable Review — refactor_cleanup

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

pelikhan commented Sep 13, 2026

Uh oh!

github-actions Bot commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Sep 13, 2026 •

edited

Loading

github-actions Bot commented Sep 13, 2026 •

edited

Loading

github-actions Bot commented Sep 13, 2026 •

edited

Loading

github-actions Bot commented Sep 13, 2026 •

edited

Loading

github-actions Bot commented Sep 13, 2026 •

edited

Loading

github-actions Bot commented Sep 13, 2026 •

edited

Loading