fix: selectively git add known files in maintenance update command#25370
fix: selectively git add known files in maintenance update command#25370
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9b04cad3-b1c3-44cf-a2c7-02ca1073a2d3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9b04cad3-b1c3-44cf-a2c7-02ca1073a2d3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
There was a problem hiding this comment.
Pull request overview
This PR updates the maintenance update/upgrade automation to stage only an explicit allowlist of “known safe” files (rather than discovering changes via git status), reducing the risk of accidentally committing temporary files.
Changes:
- Introduces update/upgrade allowlists and stages only those paths, then derives the PR file list from
git diff --cached --name-only. - Moves branch creation/commit until after staged changes are confirmed (avoids empty branches/PRs).
- Updates unit tests to reflect allowlist-based staging behavior.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/run_operation_update_upgrade.cjs | Replaces deny-list git status discovery with allowlist-based git add + staged-diff verification; delays branch creation until after staging is confirmed. |
| actions/setup/js/run_operation_update_upgrade.test.cjs | Updates mocks/assertions to validate allowlist staging and staged-diff behavior. |
| .github/workflows/refactoring-cadence.lock.yml | Quotes ${RUNNER_TEMP} script paths / adds bash invocation changes. |
| .github/workflows/design-decision-gate.lock.yml | Quotes ${RUNNER_TEMP} script paths / adds bash invocation changes. |
| .github/workflows/deep-report.lock.yml | Updates locked workflow content (quoting, prompt IDs/hashes, and an env var expression change). |
| .github/workflows/architecture-guardian.lock.yml | Quotes ${RUNNER_TEMP} script paths / adds bash invocation changes. |
| .github/workflows/approach-validator.lock.yml | Quotes ${RUNNER_TEMP} script paths / adds bash invocation changes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 3
| ".github/agents/setup-agentic-workflows.md", | ||
| ".github/agents/update-agentic-workflows.md", | ||
| ".github/agents/upgrade-agentic-workflows.md", | ||
| ".github/aw/upgrade-agentic-workflow.md", |
There was a problem hiding this comment.
KNOWN_FILES_UPGRADE includes .github/aw/upgrade-agentic-workflow.md, but this repo (and existing references) use .github/aw/upgrade-agentic-workflows.md (plural). As written, git add will fail for the intended upgrade prompt file and any changes to it won’t be included in the upgrade PR. Update the allowlist entry to the correct path (and keep any truly-deprecated paths separate if needed).
| ".github/aw/upgrade-agentic-workflow.md", | |
| ".github/aw/upgrade-agentic-workflows.md", |
| try { | ||
| await exec.exec("git", ["add", "--", file]); | ||
| } catch (error) { | ||
| core.warning(`Failed to stage '${file}': ${getErrorMessage(error)}`); | ||
| } |
There was a problem hiding this comment.
The staging loop warns on any git add failure. With the current upgrade allowlist including deprecated/old paths (e.g., .github/aw/schemas/agentic-workflow.json), many repos will hit pathspec ... did not match any files every run after those files are already removed, creating noisy action logs. Consider treating “pathspec did not match” as a non-warning (skip silently or log at info/debug), or pre-check whether the path exists/is tracked before attempting to stage it, while still allowing deletions to be staged.
| try { | |
| await exec.exec("git", ["add", "--", file]); | |
| } catch (error) { | |
| core.warning(`Failed to stage '${file}': ${getErrorMessage(error)}`); | |
| } | |
| const { exitCode, stdout, stderr } = await exec.getExecOutput( | |
| "git", | |
| ["add", "--", file], | |
| { ignoreReturnCode: true }, | |
| ); | |
| if (exitCode === 0) { | |
| continue; | |
| } | |
| const gitAddOutput = [stdout, stderr].filter(Boolean).join("\n"); | |
| if (/pathspec .* did not match any file/i.test(gitAddOutput)) { | |
| core.info(`Skipping staging for '${file}': path does not exist or is not tracked`); | |
| continue; | |
| } | |
| core.warning(`Failed to stage '${file}': ${gitAddOutput || `git add exited with code ${exitCode}`}`); |
| # poutine:ignore untrusted_checkout_exec | ||
| run: | | ||
| bash ${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh | ||
| bash "${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh" | ||
| { |
There was a problem hiding this comment.
The PR description focuses on changing the maintenance update/upgrade staging logic, but this PR also modifies several locked workflow files (mostly adding bash + quoting around ${RUNNER_TEMP} paths, and other env tweaks). Please confirm these workflow changes are intentional and update the PR description accordingly so reviewers understand the full scope.
Summary
The
run_operation_update_upgrade.cjsscript previously usedgit status --porcelainto discover all changed files, then filtered out.github/workflows/paths. This deny-list approach could accidentally stage temporary files created during theupdate/upgradeprocess.Changes
KNOWN_FILES_UPDATEallowlist: only.github/aw/actions-lock.json(the only non-workflow file thatgh aw updatecan modify)KNOWN_FILES_UPGRADEallowlist:actions-lock.json, agent files, and old deprecated agent files that may be deleted by the upgrade processgit addeach known file (warn and continue on error), then checkgit diff --cached --name-onlyfor what was actually stagedgit statusmock, added explicit allowlist behavior testsTest Results
All 20 tests pass (
npx vitest run actions/setup/js/run_operation_update_upgrade.test.cjs).Related
Fixes the issue seen at https://github.com/github/gh-aw/actions/runs/24151113577/job/704780671