Conversation
… NTFS char failures Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3a5dc581-4953-4eaa-a437-d4481a5da2e9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3a5dc581-4953-4eaa-a437-d4481a5da2e9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| shell: bash | ||
| run: | | ||
| find /tmp/gh-aw/repo-memory/default -depth -type f 2>/dev/null | while IFS= read -r f; do | ||
| dir=$(dirname "$f") | ||
| base=$(basename "$f") | ||
| safe=$(printf '%s' "$base" | sed 's/[?:*|<>"]/-/g') | ||
| if [ "$base" != "$safe" ]; then | ||
| mv -- "$f" "$dir/$safe" && echo "Renamed: $base -> $safe" | ||
| fi | ||
| done |
There was a problem hiding this comment.
@copilot repo-memory is backed by git, use git commands. Move shell code to script in actions/setup/sh
There was a problem hiding this comment.
Done in 57c4de8. Moved the sanitization to actions/setup/sh/sanitize_repo_memory_filenames.sh which uses git mv for tracked files (so the rename appears in git history) and mv for untracked files the agent wrote. The inline shell block in repo_memory.go is replaced with bash "${RUNNER_TEMP}/gh-aw/actions/sanitize_repo_memory_filenames.sh" + MEMORY_DIR env var, matching the same pattern used by clone_repo_memory_branch.sh and commit_cache_memory_git.sh.
…s.sh using git commands Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9aa6d1b9-1ab1-40c6-9621-41ec2a0e2827 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Hey One thing that would help get this across the finish line:
If you'd like a hand, you can assign this prompt to your coding agent:
|
There was a problem hiding this comment.
Pull request overview
Adds a workflow-level guard to prevent GitHub Actions artifact uploads from failing when repo-memory contains NTFS-incompatible filenames (e.g., ? : * | < > "), by sanitizing filenames prior to uploading repo-memory artifacts.
Changes:
- Emit a new “Sanitize repo-memory/wiki-memory filenames” step before each repo-memory artifact upload step.
- Introduce
actions/setup/sh/sanitize_repo_memory_filenames.shto rename tracked files viagit mvand untracked files viamv. - Recompile workflow lock files to include the new sanitization step.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/repo_memory.go | Generates an always-run sanitization step before uploading each repo-memory artifact. |
| actions/setup/sh/sanitize_repo_memory_filenames.sh | Adds a bash script to rename files with NTFS-forbidden characters before artifact upload. |
| .github/workflows/workflow-health-manager.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/weekly-blog-post-writer.lock.yml | Includes the new sanitization step prior to wiki-memory artifact upload. |
| .github/workflows/technical-doc-writer.lock.yml | Includes the new sanitization step prior to wiki-memory artifact upload. |
| .github/workflows/smoke-ci.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/security-compliance.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/pr-triage-agent.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/metrics-collector.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/glossary-maintainer.lock.yml | Includes the new sanitization step prior to wiki-memory artifact upload. |
| .github/workflows/firewall-escape.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/discussion-task-miner.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/developer-docs-consolidator.lock.yml | Includes the new sanitization step prior to wiki-memory artifact upload. |
| .github/workflows/delight.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/deep-report.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/daily-testify-uber-super-expert.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/daily-sentrux-report.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/daily-news.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/daily-community-attribution.lock.yml | Includes the new sanitization step prior to wiki-memory artifact upload. |
| .github/workflows/daily-code-metrics.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/daily-cli-performance.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-token-optimizer.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-token-audit.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-session-insights.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-pr-prompt-analysis.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-pr-nlp-analysis.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-cli-deep-research.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/copilot-agent-analysis.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/code-scanning-fixer.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload for a non-default memory ID. |
| .github/workflows/audit-workflows.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
| .github/workflows/agent-performance-analyzer.lock.yml | Includes the new sanitization step prior to repo-memory artifact upload. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/sh/sanitize_repo_memory_filenames.sh:64
- Same collision/overwrite risk exists for untracked files:
mv -- "$filepath" "$newpath"will fail if the sanitized destination already exists, and withset -euo pipefailthe whole step fails. Add the same unique-name or skip-with-warning logic here to keep sanitization best-effort.
while IFS= read -r -d '' filepath; do
base=$(basename "$filepath")
safe=$(sanitize_name "$base")
if [ "$base" != "$safe" ]; then
dir=$(dirname "$filepath")
if [ "$dir" = "." ]; then
newpath="$safe"
else
newpath="$dir/$safe"
fi
mv -- "$filepath" "$newpath"
echo "Renamed untracked: $filepath -> $newpath"
fi
done < <(git ls-files --others -z 2>/dev/null)
- Files reviewed: 31/31 changed files
- Comments generated: 4
| fmt.Fprintf(builder, " - name: Sanitize %s filenames (%s)\n", memoryLabel, memory.ID) | ||
| builder.WriteString(" if: always()\n") | ||
| builder.WriteString(" env:\n") | ||
| fmt.Fprintf(builder, " MEMORY_DIR: %s\n", memoryDir) | ||
| builder.WriteString(" run: bash \"${RUNNER_TEMP}/gh-aw/actions/sanitize_repo_memory_filenames.sh\"\n") | ||
|
|
There was a problem hiding this comment.
The sanitization step is added with if: always() but without continue-on-error: true. Since the called script uses set -euo pipefail, any rename failure (e.g., name collision) will fail this step and can fail the entire job, which defeats the purpose of preventing artifact upload failures. Consider marking this step continue-on-error: true and/or ensuring the script never exits non-zero for recoverable rename issues.
| // Step: Sanitize filenames before upload to prevent artifact upload failures. | ||
| // GitHub Actions artifacts are stored on NTFS-compatible filesystems, so filenames | ||
| // must not contain: ? : * | < > " (among other characters). | ||
| // The agent may create files with these characters (e.g. "Can-we-have-a-PR?.md"), | ||
| // which causes the upload-artifact action to fail with a hard error. | ||
| // The script uses git commands (git mv for tracked files, mv for untracked) since | ||
| // repo-memory is backed by a git working tree. | ||
| fmt.Fprintf(builder, " - name: Sanitize %s filenames (%s)\n", memoryLabel, memory.ID) | ||
| builder.WriteString(" if: always()\n") | ||
| builder.WriteString(" env:\n") | ||
| fmt.Fprintf(builder, " MEMORY_DIR: %s\n", memoryDir) | ||
| builder.WriteString(" run: bash \"${RUNNER_TEMP}/gh-aw/actions/sanitize_repo_memory_filenames.sh\"\n") | ||
|
|
||
| // Step: Upload repo-memory directory as artifact | ||
| fmt.Fprintf(builder, " - name: Upload %s artifact (%s)\n", memoryLabel, memory.ID) |
There was a problem hiding this comment.
New behavior (emitting a sanitization step before artifact upload) isn’t covered by existing repo-memory generation tests. Add a unit test asserting the generated YAML includes the sanitize step (and that it appears before the upload-artifact step) to prevent regressions when the workflow templates change.
| if [ ! -d ".git" ]; then | ||
| echo "Not a git repository: $MEMORY_DIR — skipping sanitization" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The script exits early when $MEMORY_DIR/.git is missing. However, repo-memory can be a plain directory (e.g., when clone_repo_memory_branch.sh skips git init because the branch is missing and create-orphan is false), and files in that directory can still break artifact upload. Consider still performing a best-effort find ... -depth rename with mv even when it’s not a git repo, and only use git mv when git metadata is available.
| while IFS= read -r -d '' filepath; do | ||
| base=$(basename "$filepath") | ||
| safe=$(sanitize_name "$base") | ||
| if [ "$base" != "$safe" ]; then | ||
| dir=$(dirname "$filepath") | ||
| if [ "$dir" = "." ]; then | ||
| newpath="$safe" | ||
| else | ||
| newpath="$dir/$safe" | ||
| fi | ||
| git mv -- "$filepath" "$newpath" | ||
| echo "Renamed tracked: $filepath -> $newpath" | ||
| fi |
There was a problem hiding this comment.
Renames can collide (e.g., a?.md and a*.md both sanitize to a-.md). With set -e, a failed git mv/mv due to an existing destination will exit non-zero and fail the workflow. Add collision handling (e.g., generate a unique suffix when $newpath already exists, or skip with a warning) and avoid overwriting existing files.
This issue also appears on line 51 of the same file.
|
@copilot review all comments |
…nd tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a6f0672f-efe2-4ca4-a223-74e6af700fa7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
All four reviewer comments addressed in 33ff510:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
The Glossary Maintainer agent job failed because the Copilot agent created a file named
Can-we-have-a-PR?.mdin/tmp/gh-aw/repo-memory/default/. GitHub Actions artifact upload rejects filenames containing NTFS-incompatible characters (? : * | < > "), causing a hard error that failed the entire agent job.Changes
pkg/workflow/repo_memory.go—generateRepoMemoryArtifactUpload()emits a sanitization step (withcontinue-on-error: true) before each repo-memory artifact upload. The step callssanitize_repo_memory_filenames.shvia${RUNNER_TEMP}/gh-aw/actions/and passes the memory directory viaMEMORY_DIRenv var.actions/setup/sh/sanitize_repo_memory_filenames.sh(new) — Renames files whose names contain NTFS-forbidden characters, replacing them with-:git mvfor tracked files (preserving git history) andmvfor untracked files.find+mvfor all files.a?.mdanda*.mdboth sanitizing toa-.md) by appending a numeric suffix (a--1.md), including detection of broken symlinks.Applies to both wiki and non-wiki repo-memory modes.
All 205 workflow lock files recompiled.