Skip to content

Fail activation when required skills cannot install - #58626

Merged
pelikhan merged 5 commits into
mainfrom
copilot/fix-skill-install-warning-issue
Sep 5, 2026
Merged

Fail activation when required skills cannot install#58626
pelikhan merged 5 commits into
mainfrom
copilot/fix-skill-install-warning-issue

Conversation

Copilot AI commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Declared skills could fail to install without failing the workflow, allowing the agent to run without required policy or safety guidance. Skill failures must instead stop agent execution while remaining visible in the failure issue or comment.

  • Failure handling

    • Replace the collector warning with core.setFailed(...) after publishing structured failure outputs.
    • This fails activation and prevents the dependent agent job from running.
  • Failure reporting

    • Allow the conclusion job to run when skill_install_failure_count != '0'.
    • Reuse the existing specialized skill-installation failure category and message with per-skill error details.
core.setOutput("failure_count", String(failureCount));
core.setOutput("errors", formattedErrors);

if (failureCount > 0) {
  core.setFailed(`${failureCount} skill(s) failed to install`);
}
  • Coverage
    • Verify successful installs remain non-failing.
    • Verify failed installs preserve outputs, fail activation, and enable conclusion reporting.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix skill install failure warning without job failure Fail activation when required skills cannot install Sep 4, 2026
Copilot AI requested a review from pelikhan September 4, 2026 22:54
@pelikhan
pelikhan marked this pull request as ready for review September 4, 2026 22:55
Copilot AI balanced review requested due to automatic review settings September 4, 2026 22:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

An empty failure-count output incorrectly triggers conclusion jobs for skipped or cancelled activations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Makes required skill-installation failures block agent execution while preserving structured failure reporting for issue #58446.

Changes:

  • Fails activation when skill installation errors are collected.
  • Enables conclusion reporting for skill failures.
  • Adds Go and JavaScript regression coverage.
File summaries
File Description
actions/setup/js/collect_skill_install_failures.cjs Marks collected failures as fatal.
actions/setup/js/collect_skill_install_failures.test.cjs Tests success and failure behavior.
pkg/workflow/notify_comment_conclusion_helpers.go Adds skill failures to the conclusion condition.
pkg/workflow/notify_comment_test.go Tests generated condition inclusion.
.github/workflows/ponytail-reviewer.lock.yml Regenerates the conclusion condition.
.github/workflows/mattpocock-skills-reviewer.lock.yml Regenerates the conclusion condition.
.github/workflows/impeccable-skills-reviewer.lock.yml Regenerates the conclusion condition.
.github/workflows/daily-documentation-diagram.lock.yml Regenerates the conclusion condition.
.github/workflows/archivx-agentic-workflows-analyzer.lock.yml Regenerates the conclusion condition.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +524 to +527
if len(data.SkillReferences) > 0 || len(data.Skills) > 0 {
skillInstallFailed := BuildNotEquals(BuildPropertyAccess(fmt.Sprintf("needs.%s.outputs.skill_install_failure_count", constants.ActivationJobName)), BuildStringLiteral("0"))
activationGuardrailsFailed = BuildOr(activationGuardrailsFailed, skillInstallFailed)
}
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ Security scanning failed for Ponytail Reviewer. Review the logs for details.

Lean already. Ship.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #58626

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

No ADR enforcement needed: PR does not have the implementation label and has <=100 new lines of code in business logic directories.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Comment Memory

reviewed_at: 2026-09-04T23:06:46Z
review_event: REQUEST_CHANGES
top_themes:
  - conclusion job can misfire on skipped activation because empty skill_install_failure_count satisfies != '0'
files_reviewed:
  - .github/workflows/archivx-agentic-workflows-analyzer.lock.yml
  - .github/workflows/daily-documentation-diagram.lock.yml
  - .github/workflows/impeccable-skills-reviewer.lock.yml
  - .github/workflows/mattpocock-skills-reviewer.lock.yml
  - .github/workflows/ponytail-reviewer.lock.yml
  - actions/setup/js/collect_skill_install_failures.cjs
  - actions/setup/js/collect_skill_install_failures.test.cjs
  - pkg/workflow/notify_comment_conclusion_helpers.go
  - pkg/workflow/notify_comment_test.go
comment_count: 0

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 17.5 AIC · ⌖ 7.32 AIC · ⊞ 23.5K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes

The activation-failure path is headed in the right direction, but there is still a blocking edge case in the conclusion-job condition that can fire on skipped activation and create noise on workflows that never ran.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 17.5 AIC · ⌖ 7.32 AIC · ⊞ 23.5K
Comment /review to run again

@gh-aw-bot

Copy link
Copy Markdown
Collaborator
pr-sous-chef

@copilot Please take the next forward-progress pass on this PR.

Generated by PR Sous Chef: https://github.com/github/gh-aw/actions/runs/33929489073

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 31.9 AIC · ⌖ 8.65 AIC · ⊞ 9.2K ·
Comment /souschef to run again

Copilot AI and others added 2 commits September 4, 2026 23:38
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author
pr-sous-chef
```...

Updated buildConclusionJobCondition in pkg/workflow/notify_comment_conclusion_helpers.go to require a non-empty skill failure count before checking for a non-zero value. The focused test now covers both guards. Fix committed as d0efffb.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot PR sous-chef triage: please address the blocking review feedback, refresh the branch if needed, and run the pr-finisher skill once updates are pushed. Review items currently blocking merge: #58626 (review) and #58626 (review)

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 24.2 AIC · ⌖ 8.7 AIC · ⊞ 8.9K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@copilot PR sous-chef triage: please address the blocking review feedback, refresh the branch if needed, and run the pr-finisher skill once updates are pushed. Review items currently blocking merge:...

The blocking review feedback is addressed in d0efffb: conclusion reporting now requires skill_install_failure_count to be non-empty and non-zero, preventing skipped or cancelled activations from firing the conclusion job. Targeted workflow tests pass; the branch is refreshed with the fix.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Category: bug | Risk: high | Score: 82 | Breakdown: impact 44/urgency 26/quality 12 | Action: fast_track | Batch: workflow-safety

Generated by 🔧 PR Triage Agent · copilot · mai10 · 24 AIC · ⌖ 2.63 AIC · ⊞ 15.1K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator
pr-sous-chef

@copilot Please take the next forward-progress pass on this PR.

  • The current head appears to include your conclusion-job guard fix, but blocking review state is still visible on the PR.
  • Please reconcile the current head against the remaining review items, refresh the branch if needed, and run the pr-finisher skill before handing back to maintainers.
  • If the only remaining blocker is stale review state or routine maintainer attention rather than code changes, say that explicitly.

Generated by PR Sous Chef: https://github.com/github/gh-aw/actions/runs/33937269999

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 26.3 AIC · ⌖ 8.67 AIC · ⊞ 9.2K ·
Comment /souschef to run again

Copilot AI commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author
pr-sous-chef
```...

Reconciled the current head (4c06e16) against the remaining review feedback. The requested guard is present in d0efffb, targeted tests and final local formatting/lint/unit checks pass, and no further code changes are needed. The only remaining blocker is the outdated unresolved review state plus routine maintainer review; the review thread is stale and requires maintainer resolution.

@pelikhan
pelikhan merged commit 021f2ee into main Sep 5, 2026
43 checks passed
@pelikhan
pelikhan deleted the copilot/fix-skill-install-warning-issue branch September 5, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skill install failure is only a warning, so the job succeeds without the skill

4 participants