Skip to content

Enrich dependency alerts with Copilot compatibility assessments - #33

Merged
harder merged 13 commits into
mainfrom
codex/critical-dependency-enrichment
Oct 5, 2026
Merged

harder merged 13 commits into
mainfrom
codex/critical-dependency-enrichment

Conversation

@harder

@harder harder commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

What changed

  • Enrich critical-dependency issues with upstream release highlights, skill-specific notes, explicit verification status, and focused follow-up checks. Retry missing or malformed Copilot assessments on existing issues.
  • Run pinned Copilot CLI with read-only tools for each new issue or manual reassessment. Validate required sections, compatibility status, and the 500-word limit before a separate least-privilege publisher updates its own marked comment.
  • Harden gh skill install calls: put selectors after --, pass --pin=<ref> safely, treat --upstream as a boolean flag, and place @VERSION on a skill selector rather than a repository. Update TUI and CLI call sites.
  • Add a live gh skill list --json contract check and update automation documentation.

Verification

  • Node monitor and assessment tests: 9 passed.
  • Real gh 2.102.0 contract tests: 7 passed; all 48 install-agent IDs match the catalog.
  • Focused install/parser tests: 44 passed. Release build: zero warnings or errors.
  • Live workflow dispatch for issue GitHub CLI v2.102.0 compatibility review #31 on the latest commit: monitor, Copilot assessment, and comment publishing all passed.
  • Latest PR CI: Linux/macOS/Windows tests, four AOT targets, CodeQL, workflow lint, and site check passed; remaining checks in progress.
  • Copilot PR review requested again for the latest commit after addressing prior findings.

Related: #31

@harder
harder requested a balanced review from Copilot October 5, 2026 21:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Release filtering can include unrelated notes, and one failed matrix leg suppresses all successful assessments.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds automated Copilot compatibility assessments to critical-dependency alerts while hardening gh skill install argument handling.

Changes:

  • Adds read-only Copilot assessment and publishing jobs.
  • Enriches dependency issues with release highlights and verification status.
  • Places untrusted install selectors after -- and adds contract coverage.
File Description
AGENTS.md Documents automation and argument-safety contracts.
.github/​actionlint.yaml Scopes the new permission lint exception.
.github/​copilot-instructions.md Adds dependency-assessment guidance.
.github/​scripts/​critical-dependency-prompt.md Defines the assessment prompt.
.github/​scripts/​critical-dependencies.js Generates enriched alerts and reassessment outputs.
.github/​scripts/​critical-dependencies.test.js Tests alerts, summaries, and reassessment validation.
.github/​workflows/​README.md Documents the assessment workflow.
.github/​workflows/​critical-dependencies.yml Runs and publishes Copilot assessments.
src/​SkillView.Core/​Gh/​GhSkillInstallService.cs Adds the positional option boundary.
tests/​SkillView.Tests/​Gh/​GhCliContractTests.cs Adds live JSON inventory coverage.
tests/​SkillView.Tests/​Gh/​GhSkillInstallServiceTests.cs Tests safe argument ordering.

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

Comment thread .github/scripts/critical-dependencies.js Outdated
Comment thread .github/workflows/critical-dependencies.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Version refs are still attached to repository arguments incorrectly, and assessment bounds and upstream mention handling need enforcement.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Sanitize upstream @​mentions in generated issue highlights

.github/​scripts/​critical-dependencies.js:63

Unlike releaseOverview, this copies matching upstream text into an issue without neutralizing @mentions. Release notes commonly credit contributors, so a relevant paragraph can make the monitor notify unrelated upstream users or teams from the generated SkillView issue. Apply the same mention sanitization before storing the highlight.

Medium severity Pass repository refs via --pin instead of positional parsing

src/​SkillView.Core/​Gh/​GhSkillInstallService.cs:116

gh skill install does not parse a ref from the repository positional; refs are accepted as SKILL@ref or via --pin. Consequently, a versioned listing passes owner/repo@ref to repository parsing and can fail instead of listing that ref. Since this path has no skill positional, pass --pin=<ref> before the separator and keep the repository unchanged.

This issue also appears on line 302 of the same file.

Comment thread .github/scripts/critical-dependency-assessment.js
@harder
harder requested a balanced review from Copilot October 5, 2026 21:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Versioned installs remain incompatible with gh’s argument contract, and assessment publication has validation and reliability gaps.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (1)
Previously missed (3)

In code that hasn't changed since last review

Medium severity Verdict validation scans all text instead of compatibility section

.github/​scripts/​critical-dependency-assessment.js:32

This checks for a verdict anywhere in the generated text, not specifically in ### Compatibility assessment, and does not enforce the requested single verdict. For example, a release-summary sentence containing **Likely compatible** lets an unsupported assessment such as “Probably fine” pass validation and be published. Validate exactly one allowed verdict and require it to occur in the compatibility section.

Medium severity Marker matching accepts comments from non-workflow authors

.github/​workflows/​critical-dependencies.yml:133

The marker alone does not prove that the existing comment belongs to this workflow. Any issue participant can post or quote this public marker first, causing a rerun to target an unrelated user's comment (either overwriting it or failing authorization) instead of maintaining the workflow's own assessment. Restrict the match to the GitHub Actions bot author.

Medium severity Versioned discovery passes ref as repository instead of --pin

src/​SkillView.Core/​Gh/​GhSkillInstallService.cs:116

Versioned discovery still puts @<ref> on the repository argument. In both supported gh 2.97.0 and 2.102.0, the first positional is parsed strictly as the repository, while a ref is selected via --pin <ref> (or via skill@version when a skill exists). Thus ListRepoSkillsAsync(..., "v1.2.0") asks gh for a repository literally named repo@v1.2.0 and fails instead of listing that ref. Pass the version as the trusted --pin value before the separator and keep the repository unchanged.

This issue also appears on line 301 of the same file.

Comment thread .github/scripts/critical-dependencies.js
@harder
harder requested a balanced review from Copilot October 5, 2026 21:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Versioned discovery, assessment validation, comment ownership, and release-note mention handling have unresolved defects.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Validate verdict only within the compatibility assessment section

.github/​scripts/​critical-dependency-assessment.js:32

The verdict regex scans the entire response, so malformed output is accepted when an allowed phrase appears in another section while ### Compatibility assessment contains no required verdict. Validate exactly one allowed verdict at the start of the extracted compatibility section before publishing.

Low severity Sanitize contributor mentions in copied release highlights

.github/​scripts/​critical-dependencies.js:68

Relevant excerpts are copied verbatim, so a normal generated release-note bullet such as ... by @contributor pings that upstream user when this workflow opens its issue. releaseOverview already neutralizes these mentions; apply equivalent sanitization here before embedding highlights.

Comment thread .github/workflows/critical-dependencies.yml Outdated
Comment thread src/SkillView.Core/Gh/GhSkillInstallService.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Credential exposure, comment-marker spoofing, and install argument validation defects remain unresolved.

Review effort: Balanced
Findings: 2 High severity · 1 Medium severity

Open (3)
Resolved since last review (1)
Previously missed (4)

In code that hasn't changed since last review

Medium severity Require the verdict at the start of the compatibility section

.github/​scripts/​critical-dependency-assessment.js:32

The verdict regex scans the whole assessment rather than the compatibility section. For example, an **Unknown** quoted under “What changed” lets an assessment whose compatibility section has no verdict pass validation and be published. Require exactly one allowed verdict and require it at the start of sections[2].

Medium severity Only edit bot-authored comments; otherwise create a new comment

.github/​workflows/​critical-dependencies.yml:133

The first marked comment may be user-authored. In that case the workflow tries to edit a comment it does not own, receives a permission error, and never publishes the assessment. Restrict replacement to the workflow bot's marked comment; otherwise create a new one.

Medium severity Reject mutually exclusive flags before environment probing

src/​SkillView.Core/​Cli/​CliDispatcher.cs:751

The boolean parser allows --from-local and --upstream together, and the updated parser test even constructs that combination. Every supported gh version rejects these flags as mutually exclusive, so the CLI performs environment/inventory work only to fail in the subprocess. Return invalid usage before probing, and keep this validation aligned with the TUI/service path.

Medium severity Disable upstream mode when local mode is selected

src/​SkillView.Core/​Ui/​InstallScreen.cs:313

This checkbox value is passed independently of FromLocal, so selecting both produces a command that gh rejects because --upstream and --from-local are mutually exclusive. Disable and clear the upstream checkbox while local mode is selected (and enforce the same invariant at the service/CLI boundary) instead of allowing an install that is guaranteed to fail.

Comment thread .github/workflows/critical-dependencies.yml
Comment thread .github/scripts/critical-dependencies.js Outdated
@harder
harder requested a balanced review from Copilot October 5, 2026 22:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Malformed marked assessments can suppress retries, and invalid compatibility statuses can pass validation.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Status validation accepts allowed values from unrelated sections

.github/​scripts/​critical-dependency-assessment.js:38

The allowed-status regex scans the entire output, so **Unknown** in “What changed” lets an arbitrary value such as “Probably compatible” pass in the actual compatibility section. Validate that exactly one allowed status occurs and that it belongs to sections[2] before publishing.

Comment thread .github/scripts/critical-dependencies.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

It combines model-driven issue publishing with user-visible install security changes, warranting final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (3)

@harder
harder merged commit bb46663 into main Oct 5, 2026
17 checks passed
@harder
harder deleted the codex/critical-dependency-enrichment branch October 5, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants