-
Notifications
You must be signed in to change notification settings - Fork 35
Standardize safe output references to singular "upload-asset" and separate job #7295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
Co-authored-by: pelikhan <[email protected]>
pkg/workflow/js/upload_assets.cjs
Outdated
| // Also check for legacy upload-asset items | ||
| const uploadAssetItems = result.items.filter(/** @param {any} item */ item => item.type === "upload_asset"); | ||
| // Handle legacy upload-assets (plural, hyphenated) with warning | ||
| const legacyUploadAssetsItems = result.items.filter(/** @param {any} item */ item => item.type === "upload-assets"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot remove legacy support in JavaScript. At this point, it is already normalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in c773365. The legacy handling was unnecessary since safe_outputs_handlers.cjs always generates entries with type "upload_asset" (singular), so the normalization happens before the upload processing.
Co-authored-by: pelikhan <[email protected]>
|
🤖 DIAGNOSTIC COMPLETE: Smoke Copilot No Firewall STATUS: ALL_UNITS_OPERATIONAL. MISSION_SUCCESS. |
|
🎉 Yo ho ho! Smoke Copilot Safe Inputs found the treasure and completed successfully! ⚓💰 |
|
✅ Firewall validation complete... Smoke Codex Firewall confirmed network sandboxing is operational. 🛡️ |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
Smoke Test ResultsLast 2 merged PRs:
Test Results:
Overall Status: FAIL (missing safeinputs-gh tool)
|
|
PRs: Add command/container execution support to sandbox.mcp configuration; Fix choice type conversion to JSON Schema in safe-output jobs
|
Smoke Test Results✅ File writing test - created Overall Status: FAIL (1/3 tests passed - missing safeinputs-gh tool)
|
|
Keep the processing of upload assets message in a separate job, not in safe_outputs job. Make sure to configure the git credentials |
|
@copilot comment above |
Smoke Test ResultsLast 2 merged PRs:
Test Results:
Status: PASS (5/6 tests completed successfully)
|
|
Merged PRs:
|
Co-authored-by: pelikhan <[email protected]>
Done in e937816. The upload_assets processing is now a separate job (not in the consolidated safe_outputs job) with proper git credentials configuration:
The separate job has |
|
@copilot format, lint, update dev.md to create a dummy graph with python and upload it as assets. |
Co-authored-by: pelikhan <[email protected]>
Done in c3d658c:
The dev workflow now demonstrates the complete upload-asset flow with Python-generated graphs. |
|
@copilot the upload_assets job must "needs" detection if threat detection is enabled. |
Co-authored-by: pelikhan <[email protected]>
Done in f5ea338. The upload_assets job now includes the detection job in its needs := []string{mainJobName}
if threatDetectionEnabled {
needs = append(needs, constants.DetectionJobName)
}The |
✅ Dev Hawk Report - SuccessWorkflow: #3192
Dev workflow completed successfully! 🎉 Commit AnalysisThe workflow tested the final commit:
This appears to be the concluding change in the upload-asset standardization work, ensuring that when threat detection is enabled, the upload_assets job properly depends on the detection job. The successful run validates that all the upload-asset refactoring changes work correctly together.
|
Upload-Asset Standardization - COMPLETE ✅
Make all safe output references to "upload-asset" singular, consistent across frontmatter and safe outputs processing.
All Changes Completed:
JavaScript (safe output processing)
upload_assets.cjsto only handleupload_asset(singular underscore)upload-assets(plural hyphen) - normalization happens before this pointGo Code (configuration)
safe_outputs_config.goline 133 to return "upload_asset"imports.goto check "upload-asset" instead of "upload-assets"publish_assets.goto parse "upload-asset"compiler_types.goyaml tag to "upload-asset"publish_assets_test.goupload_assets_config_test.gocheckout_persist_credentials_test.gobuildUploadAssetsStepConfigfunctionJSON Schema
Codemod Tool
Applied Codemod
.github/workflows/(19 workflows fixed)pkg/cli/workflows/.github/workflows/shared/docs/src/content/docs/reference/specs/docs/slides/Job Architecture
Code Quality
make fmtmake lintTests & Validation
Summary
Successfully standardized all references to use "upload-asset" (singular) throughout:
safe-outputs.upload-asset:(singular with hyphen)upload_asset(singular with underscore)safe_outputs_handlers.cjsbefore upload processingAll tests passing, workflows recompiled, and codemod tool available for future migrations.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.