Skip to content

Fix JSDoc return type for upload artifact handler #25929

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.

Sign up for GitHub

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

Merged
pelikhan merged 2 commits into from
Apr 12, 2026
Merged

Fix JSDoc return type for upload artifact handler #25929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38fe1b7
fix: add artifactId and artifactUrl to upload handler return type
Copilot Apr 12, 2026
e5a59b2
fix: add artifactId and artifactUrl to upload handler JSDoc return type
Copilot Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/agentics-maintenance.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ jobs:
validate_workflows:
if: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.operation == 'validate' && !github.event.repository.fork }}
runs-on: ubuntu-slim
runs-on: ubuntu-latest
permissions:
Comment on lines 282 to 285
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is described as a JSDoc return-type fix, but it also changes the runner label for validate_workflows from ubuntu-slim to ubuntu-latest. The rest of this workflow (and many others in the repo) consistently use ubuntu-slim, so this looks like an unrelated change that could alter the execution environment and permissions assumptions. Please either revert this change or justify it in the PR description (or split into a separate PR) and ensure ubuntu-latest is intended here.

Copilot uses AI. Check for mistakes.
contents: read
issues: write
Expand Down
2 changes: 1 addition & 1 deletion actions/setup/js/upload_artifact.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ async function main(config = {}) {
* @param {Object} message - The upload_artifact message from the model
* @param {Object} resolvedTemporaryIds - Map of already-resolved temporary IDs (unused here)
* @param {Map<string, any>} temporaryIdMap - Shared temp-ID map; the handler does not modify it
* @returns {Promise<{success: boolean, error?: string, skipped?: boolean, tmpId?: string, artifactName?: string, slotIndex?: number}>}
* @returns {Promise<{success: boolean, error?: string, skipped?: boolean, tmpId?: string, artifactName?: string, artifactId?: number, artifactUrl?: string, slotIndex?: number}>}
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc type marks artifactUrl as optional, but handleUploadArtifact always returns an artifactUrl property (it may be an empty string). Consider making this artifactUrl: string (non-optional) to keep the annotation accurate and preserve type-safety for callers; keep artifactId optional since it can be undefined in staged mode / when the upload API doesn't return an id.

Suggested change
* @returns {Promise<{success: boolean, error?: string, skipped?: boolean, tmpId?: string, artifactName?: string, artifactId?: number, artifactUrl?: string, slotIndex?: number}>}
* @returns {Promise<{success: boolean, error?: string, skipped?: boolean, tmpId?: string, artifactName?: string, artifactId?: number, artifactUrl: string, slotIndex?: number}>}

Copilot uses AI. Check for mistakes.
*/
return async function handleUploadArtifact(message, resolvedTemporaryIds, temporaryIdMap) {
if (slotIndex >= maxUploads) {
Expand Down
Loading