Conversation
…0-day retention - Update shared/safe-output-upload-artifact.md default retention to 30 days - Migrate audit-workflows.md, org-health-report.md, technical-doc-writer.md, poem-bot.md, deep-report.md, stale-repo-identifier.md (frontmatter + prompts) - Migrate daily-firewall-report.md, daily-performance-summary.md, api-consumption-report.md (frontmatter + significant prompt updates for artifact ID substitution in discussion content) - Update chart-embedding prompts to use upload_artifact tool with tmp_artifact_* IDs and workflow run artifact links - Recompile all 182 workflow lock files" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d11556eb-0f4b-46a1-968b-0dc573522574 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ester and unbloat-docs org-health-report and stale-repo-identifier both import shared/python-dataviz.md which still provides upload_asset, causing both tools to be present simultaneously. Replaced with docs-noob-tester and unbloat-docs which have clean migrations. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d11556eb-0f4b-46a1-968b-0dc573522574 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Per feedback:
- Replace RUNNER_TEMP with /tmp/gh-aw/ as base folder in upload_artifact.cjs
staging dir, resolver file, tool descriptions, shared workflow, and all prompts
- Change upload_artifact temporary ID format from tmp_artifact_XXXXXXXXXXXXXXXXXXXXXXXXXX
to aw_XXXXXXXX (matching the format used by other safe outputs)
- Update test expectations to match new aw_[A-Za-z0-9]{8} pattern
- Update safe_outputs_tools.json (both copies) with new paths and ID format
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/cdabdbbc-34fe-4cf8-a5e9-b5d6f8d762d8
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in dd17133:
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:
|
| ### 🔗 GitHub API Calls Trend (90 days) | ||
|
|
||
|  | ||
| 📎 **Chart: GitHub API Calls Trend** — artifact `{api_calls_trend_aw_id}` available in the [workflow run artifacts](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) |
There was a problem hiding this comment.
Fixed in 79e559c. All 5 chart references are now hyperlinks to the workflow run artifacts page, e.g.:
📎 **[Chart: GitHub API Calls Trend](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})** — artifact `{api_calls_trend_aw_id}`
| default-retention-days: 30 | ||
| max-retention-days: 30 |
There was a problem hiding this comment.
Fixed in 79e559c. Added max-uploads: 5 (to support all 5 charts) and kept default-retention-days: 30 and max-retention-days: 30 explicitly. Also added skip_archive: true to the tool call instruction in the prompt.
…tifacts - Add allow.skip-archive: true to upload-artifact config (allow single-file uploads without zip archiving) - Add max-uploads: 5 to support uploading all 5 charts (was defaulting to 1) - Update retention days to 30 (kept explicit default-retention-days: 30 and max-retention-days: 30) - Add skip_archive: true to the upload_artifact tool call instruction in prompt - Make all 5 chart artifact references hyperlinks to the workflow run artifacts page so readers can download the charts directly Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f899c209-e6db-4d6a-90d9-9f8034aaf93d 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:
|
|
@copilot refactor upload-artifact to move the configurations as field in the front matter, not a tool call. upload-artifact: do not allow these values to be set by the agent. They are fixed. Remove max-retention-days, default-retention-days fields. |
…ntmatter config The agent can no longer set `retention_days` or `skip_archive` in tool calls — these are now fixed workflow configuration. Changes: - `UploadArtifactConfig`: replace `DefaultRetentionDays`/`MaxRetentionDays`/ `ArtifactAllowConfig`/`ArtifactDefaultsConfig.SkipArchive` with two new templatable fields: `RetentionDays *string` and `SkipArchive *string` - `parseUploadArtifactConfig`: parse new fields using preprocessIntFieldAsString and preprocessBoolFieldAsString (supports GH Actions expressions) - `compiler_safe_outputs_config.go`: serialize new fields via AddTemplatableInt / AddTemplatableBool instead of old allow-skip-archive / default-retention-days - `upload_artifact.cjs`: use fixed `retention-days` and `skip-archive` from config; remove agent-controlled `retention_days` / `skip_archive` message fields; remove `clampRetention` helper - Both `safe_outputs_tools.json`: remove `retention_days` and `skip_archive` from the tool input schema (agent cannot set these) - `main_workflow_schema.json`: add `retention-days` + `skip-archive` (templatable) and remove `default-retention-days`, `max-retention-days`, `allow`, and `defaults.skip-archive` - `shared/safe-output-upload-artifact.md`: update defaults - All 9 migrated workflows + smoke-copilot.md: replace old frontmatter fields with new ones; remove `retention_days`/`skip_archive` from tool call instructions - Update Go and JS test files to cover new API Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c048f9ab-1d7e-49df-8dc0-24c7eec06584 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c048f9ab-1d7e-49df-8dc0-24c7eec06584 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in e462f5e (with 016cbca fixing a review issue). The upload-artifact:
retention-days: 30 # templated int — fixed, agent cannot override
skip-archive: true # templated bool — fixed, agent cannot override
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:
|
|
@copilot merge main and recompile |
…c-workflows-upload-artifact # Conflicts: # .github/workflows/api-consumption-report.lock.yml # .github/workflows/audit-workflows.lock.yml # .github/workflows/daily-firewall-report.lock.yml # .github/workflows/daily-performance-summary.lock.yml # .github/workflows/deep-report.lock.yml # .github/workflows/docs-noob-tester.lock.yml # .github/workflows/poem-bot.lock.yml # .github/workflows/smoke-copilot.lock.yml # .github/workflows/technical-doc-writer.lock.yml # .github/workflows/unbloat-docs.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates a subset of agentic workflows from upload-asset to upload-artifact, and refactors upload-artifact so retention/archiving behavior is fixed by workflow configuration rather than agent-provided tool inputs.
Changes:
- Refactors
upload-artifactconfig to use fixed, templatableretention-days/skip-archivefields and removes agent-settableretention_days/skip_archivetool inputs. - Updates the
upload_artifacthandler and tool schemas to use/tmp/gh-aw/...staging and the newaw_XXXXXXXXtemporary ID format. - Migrates multiple workflows (and their lockfiles) from
upload-assettoupload-artifactwith updated staging/instructions.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/publish_artifacts.go | Updates compiler-side upload-artifact config parsing/types for fixed retention/skip-archive. |
| pkg/workflow/publish_artifacts_test.go | Adjusts tests for new upload-artifact config fields and templatable support. |
| pkg/workflow/js/safe_outputs_tools.json | Removes agent inputs for retention/archiving; updates staging path text. |
| pkg/workflow/compiler_safe_outputs_config.go | Emits new handler config keys (retention-days, skip-archive) into handler config JSON. |
| pkg/parser/schemas/main_workflow_schema.json | Updates workflow schema to new upload-artifact config fields and removes deprecated ones. |
| actions/setup/js/upload_artifact.cjs | Enforces fixed retention/skip-archive from config; changes staging dir and temp ID format. |
| actions/setup/js/upload_artifact.test.cjs | Updates handler tests for new config behavior and aw_ IDs. |
| actions/setup/js/safe_outputs_tools.json | Mirrors tool schema updates in actions copy (removes retention_days/skip_archive inputs). |
| .github/workflows/shared/safe-output-upload-artifact.md | Updates shared documentation to new staging path and aw_* IDs. |
| .github/workflows/unbloat-docs.md | Migrates workflow frontmatter/instructions to upload-artifact staging + aw_* IDs. |
| .github/workflows/unbloat-docs.lock.yml | Regenerates compiled workflow to remove upload_assets job and wire upload_artifact staging/artifact flow. |
| .github/workflows/technical-doc-writer.md | Migrates workflow frontmatter from upload-asset to upload-artifact (retention-days). |
| .github/workflows/technical-doc-writer.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/smoke-copilot.md | Updates smoke test instructions to stage to upload-artifact and omit agent retention/skip params. |
| .github/workflows/smoke-copilot.lock.yml | Regenerates compiled workflow for new upload_artifact config keys and outputs. |
| .github/workflows/poem-bot.md | Migrates workflow frontmatter from upload-asset to upload-artifact (retention-days). |
| .github/workflows/docs-noob-tester.md | Migrates workflow frontmatter + instructions to stage/upload screenshots as artifacts with aw_* IDs. |
| .github/workflows/docs-noob-tester.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/deep-report.md | Migrates workflow frontmatter from upload-asset to upload-artifact (retention-days). |
| .github/workflows/deep-report.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/daily-performance-summary.md | Migrates chart publishing instructions from asset URLs to artifact IDs + run artifacts link. |
| .github/workflows/daily-performance-summary.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/daily-firewall-report.md | Migrates chart publishing instructions from asset URLs to artifact IDs + run artifacts link. |
| .github/workflows/daily-firewall-report.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/audit-workflows.md | Migrates workflow frontmatter + instructions from upload-asset to upload-artifact. |
| .github/workflows/audit-workflows.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
| .github/workflows/api-consumption-report.md | Migrates charts from embedded asset URLs to artifact IDs; bumps max-uploads to 5. |
| .github/workflows/api-consumption-report.lock.yml | Regenerates compiled workflow for new safe-outputs toolset and staging artifact flow. |
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)
.github/workflows/technical-doc-writer.lock.yml:1431
- The safe_outputs job downloads the staging artifact into
${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/, but the upload_artifact handler now reads from/tmp/gh-aw/safeoutputs/upload-artifacts/. Unless there’s a copy/symlink step (none appears nearby), the handler won’t see the staged files. Download the staging artifact into the directory the handler uses (or change the handler to match) and regenerate the lock file(s).
- name: Download upload-artifact staging
continue-on-error: true
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
name: safe-outputs-upload-artifacts
path: ${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/
- name: Process Safe Outputs
- Files reviewed: 29/29 changed files
- Comments generated: 5
| async function main(config = {}) { | ||
| const maxUploads = typeof config["max-uploads"] === "number" ? config["max-uploads"] : 1; | ||
| const defaultRetentionDays = typeof config["default-retention-days"] === "number" ? config["default-retention-days"] : 7; | ||
| const maxRetentionDays = typeof config["max-retention-days"] === "number" ? config["max-retention-days"] : 30; | ||
| // retention-days and skip-archive are fixed workflow configuration; the agent cannot override them. | ||
| const retentionDays = typeof config["retention-days"] === "number" ? config["retention-days"] : 30; | ||
| const skipArchive = config["skip-archive"] === true; | ||
| const maxSizeBytes = typeof config["max-size-bytes"] === "number" ? config["max-size-bytes"] : 104857600; |
There was a problem hiding this comment.
upload_artifact.cjs treats config["retention-days"] as a number and config["skip-archive"] as a boolean, but templatable fields are passed through the handler-config JSON as strings when they originate from GitHub Actions expressions (e.g. ${{ inputs.retention }} becomes "14"). As written, string values will be ignored (defaulting retention to 30 and skip-archive to false), breaking the advertised templatable behavior. Consider parsing these fields with the existing templatable helpers (e.g., parseIntTemplatable / parseBoolTemplatable) or accepting numeric/"true"/"false" strings.
See below for a potential fix:
* Parse an integer-like config value, accepting both numbers and numeric strings.
*
* @param {unknown} value
* @param {number} defaultValue
* @returns {number}
*/
function parseIntegerConfig(value, defaultValue) {
if (typeof value === "number" && Number.isFinite(value)) {
return value;
}
if (typeof value === "string" && value.trim() !== "") {
const parsed = Number(value);
if (Number.isFinite(parsed)) {
return parsed;
}
}
return defaultValue;
}
/**
* Parse a boolean-like config value, accepting booleans and "true"/"false" strings.
*
* @param {unknown} value
* @param {boolean} defaultValue
* @returns {boolean}
*/
function parseBooleanConfig(value, defaultValue) {
if (typeof value === "boolean") {
return value;
}
if (typeof value === "string") {
const normalized = value.trim().toLowerCase();
if (normalized === "true") {
return true;
}
if (normalized === "false") {
return false;
}
}
return defaultValue;
}
/**
* Returns a per-message handler function that processes a single upload_artifact request.
*
* @param {Object} config - Handler configuration from the safe outputs config
* @returns {Promise<Function>} Per-message handler function
*/
async function main(config = {}) {
const maxUploads = parseIntegerConfig(config["max-uploads"], 1);
// retention-days and skip-archive are fixed workflow configuration; the agent cannot override them.
const retentionDays = parseIntegerConfig(config["retention-days"], 30);
const skipArchive = parseBooleanConfig(config["skip-archive"], false);
const maxSizeBytes = parseIntegerConfig(config["max-size-bytes"], 104857600);
const defaultIfNoFiles = typeof config["default-if-no-files"] === "string" ? config["default-if-no-files"] : "error";
const allowedPaths = Array.isArray(config["allowed-paths"]) ? config["allowed-paths"] : [];
const filtersInclude = Array.isArray(config["filters-include"]) ? config["filters-include"] : [];
const filtersExclude = Array.isArray(config["filters-exclude"]) ? config["filters-exclude"] : [];
const isStaged = parseBooleanConfig(config["staged"], false) || process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true";
| // Parse common base fields (max, github-token, staged). | ||
| c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 0) | ||
|
|
||
| publishArtifactsLog.Printf("Parsed upload-artifact config: max_uploads=%d, default_retention=%d, max_retention=%d, max_size_bytes=%d", | ||
| config.MaxUploads, config.DefaultRetentionDays, config.MaxRetentionDays, config.MaxSizeBytes) | ||
| publishArtifactsLog.Printf("Parsed upload-artifact config: max_uploads=%d, retention_days=%v, skip_archive=%v, max_size_bytes=%d", | ||
| config.MaxUploads, config.RetentionDays, config.SkipArchive, config.MaxSizeBytes) |
There was a problem hiding this comment.
The PR description/docs/tool schemas say the artifact staging directory is /tmp/gh-aw/safeoutputs/upload-artifacts/, and the JS handler now reads from that path. However, the compiler still hardcodes the staging transfer path via artifactStagingDirExpr as ${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/, which is what ends up in the generated lock workflows. This mismatch means staged files can be uploaded/downloaded to a different directory than the handler reads, causing uploads to fail. Align the compiler-generated staging path with the handler (or vice versa) and regenerate the lock files.
| publishArtifactsLog.Printf("Parsed upload-artifact config: max_uploads=%d, retention_days=%v, skip_archive=%v, max_size_bytes=%d", | ||
| config.MaxUploads, config.RetentionDays, config.SkipArchive, config.MaxSizeBytes) |
There was a problem hiding this comment.
This log line prints RetentionDays / SkipArchive as %v on *string, which will log pointer addresses rather than the underlying values. For readability/debuggability, consider logging dereferenced values (with explicit nil handling) or using a helper that formats templatable strings consistently.
| "retention-days": { | ||
| "description": "Artifact retention period in days (fixed; the agent cannot override this value). Supports integer or GitHub Actions expression (e.g. '${{ inputs.retention-days }}').", | ||
| "oneOf": [ | ||
| { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| "maximum": 90 | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "pattern": "^\\$\\{\\{.*\\}\\}$" | ||
| } | ||
| ] | ||
| }, | ||
| "max-retention-days": { | ||
| "type": "integer", | ||
| "description": "Maximum retention cap in days; model requests are clamped to this value (default: 30)", | ||
| "minimum": 1, | ||
| "maximum": 90, | ||
| "default": 30 | ||
| "skip-archive": { | ||
| "description": "Upload files directly without zip archiving (fixed; the agent cannot override this value). Only valid for single-file uploads. Supports boolean or GitHub Actions expression (e.g. '${{ inputs.skip-archive }}').", | ||
| "oneOf": [ | ||
| { | ||
| "type": "boolean" | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "pattern": "^\\$\\{\\{.*\\}\\}$" | ||
| } |
There was a problem hiding this comment.
The examples in the retention-days / skip-archive descriptions use ${{ inputs.retention-days }} and ${{ inputs.skip-archive }}. In GitHub Actions expressions, inputs.retention-days is parsed as subtraction; for hyphenated input names you need bracket syntax like inputs['retention-days'] (or use underscore input names). Updating the examples avoids copy/paste breakage.
| name: safe-outputs-assets | ||
| path: /tmp/gh-aw/safeoutputs/assets/ | ||
| name: safe-outputs-upload-artifacts | ||
| path: ${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/ |
There was a problem hiding this comment.
This workflow uploads the upload-artifact staging directory from ${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/, but the upload_artifact handler/tooling in this PR expects staging under /tmp/gh-aw/safeoutputs/upload-artifacts/. If the agent follows the prompt/docs and stages into /tmp/gh-aw/..., this step will upload an empty directory and artifacts won’t be available in the safe_outputs job. Align this path with the handler’s staging directory (and regenerate if this file is generated).
This issue also appears on line 1425 of the same file.
| path: ${{ runner.temp }}/gh-aw/safeoutputs/upload-artifacts/ | |
| path: /tmp/gh-aw/safeoutputs/upload-artifacts/ |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/96cd0582-bdd1-4d3a-bc0c-d8d19d3d818c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 3f57dd5. Merged main (lock file conflicts resolved by recompiling), rebuilt the binary, and recompiled all 187 workflow files successfully. 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:
|
Summary
Migrates 9 of 17 agentic workflows (~53%) that use
upload-assetsafe output to useupload-artifactsafe output instead. Refactors theupload-artifactconfiguration so thatretention-daysandskip-archiveare fixed frontmatter fields rather than agent-settable tool call parameters.Changes
upload-artifactconfiguration refactoring (all workflows)retention-daysandskip-archiveare now fixed workflow configuration — the agent cannot override them:default-retention-daysandmax-retention-daysfields removedallow.skip-archiveanddefaults.skip-archiveremovedretention_daysandskip_archiveremoved from theupload_artifacttool input schema (bothsafe_outputs_tools.jsoncopies) — agents can no longer pass these valuesupload_artifact.cjshandler now reads fixed values from config;skip-archiveenforces a single-file constraint but does not alter the upload API callmain_workflow_schema.jsonupdated with newretention-days(templatable int) andskip-archive(templatable bool) propertiesUploadArtifactConfig) and compiler updated accordinglyshared/safe-output-upload-artifact.mdretention-days: 30,skip-archive: true,max-uploads: 3/tmp/gh-aw/safeoutputs/upload-artifacts/upload_artifact.cjs(core handler)/tmp/gh-aw/aw_XXXXXXXX(8 alphanumeric), matching other safe outputs such ascreate_issue/tmp/gh-aw/artifact-resolver.jsonsafe_outputs_tools.json(both copies)retention_daysandskip_archivefrom tool input schemaMigrated Workflows (9/17 ≈ 50%)
Group 1 — Frontmatter + minimal prompt update:
audit-workflows.md—retention-days: 30technical-doc-writer.md—retention-days: 30poem-bot.md—retention-days: 30deep-report.md—retention-days: 30Group 2 — Frontmatter + chart artifact ID substitution:
daily-firewall-report.md—retention-days: 30,skip-archive: true; chart upload instructions updateddaily-performance-summary.md—retention-days: 30,skip-archive: true; chart upload instructions updatedapi-consumption-report.md—retention-days: 30,skip-archive: true,max-uploads: 5; all 5 chart artifact references are hyperlinks to the workflow run artifacts pageGroup 3 — Frontmatter + screenshot artifact ID substitution:
docs-noob-tester.md—retention-days: 30,skip-archive: trueunbloat-docs.md—retention-days: 30,skip-archive: trueAlso updated
smoke-copilot.md(pre-existing workflow) — migrated from olddefault-retention-days/max-retention-days/allow.skip-archiveto newretention-days: 1,skip-archive: trueNot Migrated (8/17 workflows)
portfolio-analyst.md,python-data-charts.md,daily-news.md,daily-repo-chronicle.md— heavily rely on inline chart image embedding (requires permanent URLs)weekly-editors-health-check.md— embeds screenshots in MDX documentation files (requires permanent URLs)org-health-report.md,stale-repo-identifier.md— importshared/python-dataviz.mdwhich still providesupload_asset, causing conflicting tool availabilityHow the Prompt Changes Work
For chart-embedding workflows, the pattern changes from:
To:
The
aw_*temporary IDs are consistent with other safe outputs, and chart names link directly to the workflow run artifacts page for download.