feat(scripts): detect PowerShell 7-only syntax in hook manifest commands

[Jamie Kim (jkim323)]

Pull Request

Description

Test-HookManifest now rejects PowerShell 7-only syntax in any powershell hook command, so a manifest that Windows PowerShell 5.1 hosts cannot parse fails validation instead of failing silently at runtime.

Why this guard exists

The repository pins PowerShell 7 everywhere it controls invocation — 90 files declare #Requires -Version 7.4, every npm script calls pwsh, and CI uses shell: pwsh. Hook commands are the exception: the host chooses the interpreter, and it may launch the powershell branch with Windows PowerShell 5.1 (powershell.exe). A PS7-only construct there fails at parse time, before the hook script is ever reached, so the hook silently never runs and the host emits a parser error per lifecycle event.

That is not hypothetical. .github/hooks/shared/telemetry.json shipped exactly this defect across all 11 of its lifecycle events (#2724), and nothing in the toolchain caught it. docs/contributing/hooks.md keeps the hook manifest contract supported and lint:hooks runs in both validate:local and plugin:validate, so any future manifest can reintroduce the same class of defect.

What the check does

Detection matches token kinds from [System.Management.Automation.Language.Parser]::ParseInput, not raw text. Operators appearing inside string literals are therefore not false-flagged, and bash commands are never inspected.

Covered constructs:

Token kind	Construct
QuestionMark	ternary ? :
QuestionQuestion	null-coalescing ??
QuestionQuestionEquals	null-coalescing assignment ??=
QuestionDot	null-conditional member access ?.
QuestionLBracket	null-conditional index ?[
AndAnd	pipeline chain &&
OrOr	pipeline chain ||

A rejected manifest reports which construct was found and why it matters, for example:

hooks.stop[0] property 'powershell' uses PowerShell 7-only syntax (ternary operator '? :');
Windows PowerShell 5.1 hosts fail to parse it

Scope change since this PR opened

This PR originally also rewrote the ternary in .github/hooks/shared/telemetry.json to an equivalent if-expression. #2719 has since landed on main and retired the telemetry hook entirely, deleting that manifest and all 17 of its implementation files. Merging main therefore resolved the modify/delete conflict by accepting the deletion — the manifest cannot be fixed because it no longer exists, and restoring it would reintroduce a hook pointing at a removed collector script.

The preventive half is what remains, and it was always the durable half. The original PR description anticipated this outcome:

If #2717 lands first, the manifest half of this PR is superseded; the validator guard and its tests remain valuable regardless, since they protect any current or future hook manifest.

The repository now ships zero hook manifests, so this check is purely forward-looking. It costs nothing at rest — lint:hooks reports 0 found — and it fails closed the moment someone adds a manifest with the defect that #2724 documented.

Related Issue(s)

Closes #2724

Note

#2724 has two halves. The defect half (the shipped ternary) was resolved by #2719 deleting the manifest. This PR closes the recurrence half by making the toolchain reject the construct. If you would rather keep #2724 open or close it against #2719 instead, change this line before merging.

Type of Change

Select all that apply:

Code & Documentation:

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Breaking change (fix or feature causing existing functionality to change)
• Documentation update

Infrastructure & Configuration:

• GitHub Actions workflow
• Linting configuration (markdown, PowerShell, etc.)
• Security configuration
• DevContainer configuration
• Dependency update

AI Artifacts:

• Reviewed contribution with hve-builder and addressed all actionable findings
• Copilot instructions (.github/instructions/*.instructions.md)
• Copilot prompt (.github/prompts/*.prompt.md)
• Copilot agent (.github/agents/*.agent.md)
• Copilot skill (.github/skills/*/SKILL.md)
• Copilot hook (.github/hooks/*/*.json)
• Eval spec added/updated for changed AI artifacts (evals/) (N/A — no prompt, agent, instruction, or skill behavior changed)

Other:

• Script/automation (.ps1, .sh, .py)
• Other (please describe):

Testing

Diff is two files, both under scripts/:

File	Change
scripts/linting/Validate-HookManifests.ps1	Adds Get-HookPowerShellIncompatibility and wires it into Test-HookManifest (+65)
scripts/tests/linting/Validate-HookManifests.Tests.ps1	Adds the Windows PowerShell 5.1 compatibility Describe block (+41)

Automated validation re-run locally after merging main:

Command	Result
npm run plugin:validate	Pass — manifest and catalog in sync (55 agents, 48 commands, 57 rules, 73 skills); lint:hooks reports 0 found
npm run test:ps -- -TestPath "scripts/tests/linting/Validate-HookManifests.Tests.ps1"	Pass — 32/32
npm run test:ps -- -TestPath "scripts/tests/linting/"	1227 passed, 2 failed (unrelated — see below)
Invoke-ScriptAnalyzer with scripts/linting/PSScriptAnalyzer.psd1 on both changed files	Pass — 0 issues

Earlier targeted verification of the construct, performed against the manifest before #2719 removed it, is retained here as evidence that the detector matches real host behavior rather than an assumed grammar:

• Reproduced the defect. Parsing the pre-fix sessionStart command under Windows PowerShell 5.1.26100.8875 returns FAIL: Unexpected token '?' in expression or statement.
• Verified the guard fires. Against the pre-fix manifest it flagged all 11 lifecycle events; against the corrected form lint:hooks reported OK.

New coverage in Validate-HookManifests.Tests.ps1 (9 cases): ternary rejection, if-expression acceptance, no false positive on PS7-only operators inside string literals, bash commands not inspected, and one case per PS7-only construct.

Manual testing was not performed beyond the parser checks above.

Unrelated test failures: the 2 failures are both in Validate-DesignIntent.Tests.ps1 (inadequate-deciding-claim and schema-violation fixtures), which touches no file in this PR and is pre-existing — see #2722.

Checklist

Required Checks

• Documentation is updated (if applicable) (N/A — docs/contributing/hooks.md documents the manifest contract and schema, not per-construct validator rules; the rejection message is self-describing)
• Files follow existing naming conventions
• Changes are backwards compatible (if applicable)
• Tests added for new functionality (if applicable)

AI Artifact Contributions

• Used hve-builder review mode to review contribution
• Addressed all actionable findings from the hve-builder review
• Verified contribution follows common standards and type-specific requirements

Required Local Checks

The following local-safe validation commands must pass before merging:

• Local validation aggregate: npm run validate:local — not re-run in full after the merge; the lanes that own this diff (lint:ps, lint:hooks, lint:plugin-manifest) were run individually and pass. The prior full run failed only lint:md-links, on two external URLs unrelated to this diff.
• Documentation validation (if docs changed): npm run validate:docs (N/A — no docs changed)
• Spell checking: npm run spell-check — prior run reported 37 issues, all the same token, all inside the gitignored docs/docusaurus/test-results/** artifacts. No tracked file and no file in this diff is affected.
• Link validation: npm run lint:md-links (N/A — no markdown changed)

Security Considerations

• This PR does not contain any sensitive or NDA information
• Any new dependencies have been reviewed for security issues (N/A — no dependency changes)
• Security-related scripts follow the principle of least privilege (N/A — changes are in scripts/linting/, not scripts/security/)

The detector calls Parser::ParseInput, which builds an AST and returns tokens. It does not invoke Invoke-Expression, dot-source, or otherwise execute the manifest command string, so validating an untrusted manifest does not run its contents.

Additional Notes

Superseded content. The manifest edit this PR opened with is gone from the diff; git diff origin/main HEAD now shows only the two scripts/ files. The merge commit resolving that conflict is 7018fd50.

Corrections to the issue body. Two items in #2724 were stale and were not actionable:

• It lists scripts/plugins/Modules/PluginHelpers.psm1 and scripts/tests/plugins/PluginHelpers.Materialization.Tests.ps1 as requiring sync. Both were removed by fix(build): source plugin metadata from repository root #2736 ("source plugin metadata from repository root").
• Its proposed snippet uses a '.github' fallback with a hooks/... child path, which no longer matches any file on main.

[github-actions]

Eval Execution

⚠️ No eval summary was produced.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.06%. Comparing base (3c3dc02) to head (a2598a0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
scripts/linting/Validate-HookManifests.ps1	95.45%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2757      +/-   ##
==========================================
+ Coverage   82.98%   83.06%   +0.07%     
==========================================
  Files         183      169      -14     
  Lines       33787    32879     -908     
  Branches       25        0      -25     
==========================================
- Hits        28038    27310     -728     
+ Misses       5746     5569     -177     
+ Partials        3        0       -3     

Flag	Coverage Δ
docusaurus	?
pester	83.86% <95.45%> (+0.39%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
scripts/linting/Validate-HookManifests.ps1	86.71% <95.45%> (+1.58%)	⬆️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Katrien De Graeve (katriendg)]

Thank you, this implementation looks complete.