Skip to content

Fix Wimlib links and install review skills - #25

Open
vartaxe wants to merge 18 commits into
mainfrom
copilot/review-and-skills-2026-10-17
Open

vartaxe wants to merge 18 commits into
mainfrom
copilot/review-and-skills-2026-10-17

Conversation

@vartaxe

@vartaxe vartaxe commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Install github-actions-hardening, pester-should-migration, docs-sync-audit, github-release, and batch-files.
  • Correct five broken relative links in the vendored Wimlib README files.
  • Fix documentation-audit false positives for skill-relative scripts, code spans/fences, PowerShell environment reads, and URL-encoded paths.

Validation

  • Wimlib link targets verified; 20 core-file manifest entries verified.
  • PowerShell and XAML parse validation passed; focused docs-drift test and git diff --check passed.
  • Pester 6.2.0 was unavailable in this execution environment, so the repository Pester suite was not run.

Correct five broken Wimlib README links, install the selected review skills, and include the shared documentation-audit checker fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟡 Changes recommended

Broken batch control flow and unsafe temporary-directory cleanup remain, alongside audit and instructional correctness issues.

10 open issues
What changed in this PR

Adds development and review skills to the Driver Automation Tool repository and repairs vendored documentation links.

Changes:

  • Adds five skills with reference guides and batch templates.
  • Introduces documentation-drift checks and focused regression coverage.
  • Corrects five Wimlib README links.
File Description
Driver Automation Tool/​Tools/​Wimlib/​README.WINDOWS.txt Corrects the main README link.
Driver Automation Tool/​Tools/​Wimlib/​README.txt Corrects bundled documentation links.
.github/​skills/​pester-should-migration/​SKILL.md Adds assertion migration guidance.
.github/​skills/​pester-should-migration/​references/​assertion-map.md Maps classic assertions to v6 forms.
.github/​skills/​github-release/​SKILL.md Adds a release preparation workflow.
.github/​skills/​github-release/​references/​semver-rules.md Documents versioning decisions.
.github/​skills/​github-release/​references/​commit-classification.md Guides changelog classification.
.github/​skills/​github-actions-hardening/​SKILL.md Adds workflow security review guidance.
.github/​skills/​github-actions-hardening/​references/​triggers-and-privilege.md Explains trigger trust boundaries.
.github/​skills/​github-actions-hardening/​references/​supply-chain.md Covers action and runner security.
.github/​skills/​github-actions-hardening/​references/​report-format.md Defines security report formatting.
.github/​skills/​github-actions-hardening/​references/​permissions-and-tokens.md Documents token and secret handling.
.github/​skills/​github-actions-hardening/​references/​injection.md Explains script-injection prevention.
.github/​skills/​docs-sync-audit/​tests/​test_docs_drift.py Adds false-positive regression coverage.
.github/​skills/​docs-sync-audit/​SKILL.md Defines documentation audit procedures.
.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py Implements documentation-drift checks.
.github/​skills/​batch-files/​SKILL.md Adds batch scripting guidance.
.github/​skills/​batch-files/​references/​windows-subsystem-on-linux.md Collects WSL resources.
.github/​skills/​batch-files/​references/​tools-and-resources.md Lists Windows administration tools.
.github/​skills/​batch-files/​references/​msys2.md Adds MSYS2 references.
.github/​skills/​batch-files/​references/​cygwin.md Adds Cygwin integration guidance.
.github/​skills/​batch-files/​references/​batch-files-and-functions.md Documents batch scripting techniques.
.github/​skills/​batch-files/​assets/​task.txt Adds an automation task template.
.github/​skills/​batch-files/​assets/​library.txt Adds a reusable function template.
.github/​skills/​batch-files/​assets/​executable.txt Adds a command-line tool template.

Review effort: Balanced

Comment thread .github/skills/batch-files/SKILL.md Resolved
Comment thread .github/skills/github-release/SKILL.md Resolved
vartaxe and others added 2 commits November 1, 2026 01:38
Correct batch templates, docs auditing, Actions examples, release, and Pester migration guidance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Unresolved issues include broken batch argument parsing, masked failure exit codes, inaccurate documentation-audit findings, and recovery instructions that leave a persistent authentication bypass.

1 open issue
12 previously missed issues

In code that hasn't changed since last review

Medium severity Return a failure status for invalid input after cleanup

.github/​skills/​batch-files/​assets/​executable.txt:89

The no-input and missing-file branches print errors without setting a failure status, so successful cleanup can return success for rejected input. Callers checking the exit code cannot detect the failure. Track an explicit result code, set it on validation failures, and return it after cleanup.

Medium severity Trim without FOR /F to preserve leading spaces and exclamation marks

.github/​skills/​batch-files/​assets/​library.txt:84

FOR /F skips semicolon-prefixed values such as ;Hello, leaving their leading spaces, and assigning %%a with delayed expansion enabled can strip literal exclamation marks. Trim directly from the variable rather than parsing it through FOR /F, and preserve literal characters when returning the value to the caller.

Medium severity Generate enough padding for widths larger than the fixed suffix

.github/​skills/​batch-files/​assets/​library.txt:175

Appending a fixed 50 spaces cannot satisfy widths larger than the input length plus 50. For example, padRight x 100 emits only 51 characters because substring expansion cannot extend a shorter string. Generate enough padding for the requested width before slicing.

Medium severity Save and return the task result after cleanup

.github/​skills/​batch-files/​assets/​task.txt:89

The task result is not saved before logging and cleanup, which can overwrite a failure status and make Task Scheduler report success. The missing-target branch at lines 108–110 also never returns a failure code. Make validation failures return nonzero, capture the result immediately after _runMyTask, and return that saved result after cleanup.

Medium severity Use a timestamp fallback when WMIC is unavailable

.github/​skills/​batch-files/​assets/​task.txt:138

Every log call requires WMIC, but this skill does not list it as a prerequisite and its own Windows-command reference notes that it is disabled by default on Windows 11. On machines without WMIC, logging prints a command-not-found error and formats an unset _dtMyTask, producing invalid timestamps. Use PowerShell's locale-independent Get-Date or detect WMIC and provide a fallback.

Medium severity Check script paths only in executable command positions

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:68

This matches every whitespace-prefixed ./... token, including arguments rather than just executed scripts. A valid documented command such as python ./scripts/render.py --output ./out/report.json produces a high-severity missing-script-file finding for the output file, even though it is supposed to be created by the command. Restrict script-path checking to executable/interpreter positions instead of treating output and other arguments as scripts.

Medium severity Parse multiline Python imports to detect referenced modules

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:75

The import-name capture stops at the first newline, so a normal parenthesized import such as from package import ( followed by config, on the next line never adds config to the referenced names. package/config.py is then marked unreferenced, and build() reports any settings read there as unable to take effect even though the module is imported. Handle multiline import lists, preferably using Python's ast.ImportFrom nodes for Python files.

Medium severity Match Markdown fence character and length in command scanning

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:180

The command scanner uses a different fence parser from the link scanner: it ignores ~~~ fences and toggles on every triple-backtick prefix, including shorter fences embedded inside a four-backtick example. Consequently documented commands can be skipped, or prose after an incorrectly recognized closing fence can be audited as commands. Track the opening fence character and length and require a matching closing fence.

Medium severity Track multiline inline-code spans when checking links

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:201

Inline code spans can cross line boundaries, but this substitution handles each line independently. For a valid span containing a newline followed by [Fake](missing.md), build() still emits a high-severity broken-link finding for that literal example. Track matching backtick delimiters across lines while preserving line numbers, so multiline code spans receive the same exclusion as single-line spans.

Medium severity Recognize explicitly documented environment variable references

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:308

Explicitly documenting $env:windir and $env:COMPUTERNAME still produces undocumented-env findings because documentation matching requires uppercase names containing underscores. Recognize explicit $env: references and normalize them like code reads before applying the generic prose heuristic.

Low severity Remove recovery steps that create a persistent authentication bypass

.github/​skills/​batch-files/​references/​batch-files-and-functions.md:69

This recovery advice leaves an unauthenticated pre-login SYSTEM shell and creates an administrator account with a published fixed password, without restoring the modified sign-in binary. Following it on an endpoint introduces a persistent authentication bypass. Remove those instructions and direct users to an authorized, supported recovery or password-reset process instead.

Low severity Define how prerelease tags affect version selection

.github/​skills/​github-release/​SKILL.md:244

Both tag-discovery snippets accept prerelease tags such as v2.0.0-rc.1, but that value cannot be split into three integer components as instructed here. The workflow does not define whether to promote the prerelease to 2.0.0 or bump from the last stable release, so following this step can calculate the wrong version. Explicitly handle prerelease tags and define the promotion policy, or select only stable tags and treat the absence of a stable tag as the first-release case.

9 resolved since last review

Review effort: Balanced

vartaxe and others added 2 commits November 1, 2026 03:01
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Unresolved correctness issues cause inaccurate documentation audits around working directories and make targets/options, while batch helpers lose literal exclamation marks and return success for unknown functions.

No open issues 🎉

5 previously missed issues

In code that hasn't changed since last review

Medium severity Return a nonzero status for unknown functions

.github/​skills/​batch-files/​assets/​library.txt:71

An unknown function prints an error but reaches cleanup without setting a failure status. With an initially successful caller, it returns success, so callers using && or if errorlevel cannot detect a misspelled function name. Run cleanup and explicitly return a nonzero exit code on this branch.

Medium severity Use delayed expansion to preserve exclamation marks

.github/​skills/​batch-files/​assets/​library.txt:193

Delayed expansion is enabled here, but percent expansion inserts the input into the command before the exclamation-mark scan. Consequently, call myLib padRight "Hello!" 10 loses the literal ! before padding. Read the stored argument through delayed expansion instead; characters inside the expanded value are not scanned again.

Medium severity Keep Makefile targets scoped to their directories

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:173

Targets from every Makefile are merged, so a root-level make clean passes the audit when only tools/Makefile defines clean. Keep target inventories keyed by directory and check the Makefile selected by each invocation, including cd and make -C.

Medium severity Parse make options separately from targets

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:442

CMD_MAKE treats the first token after make as a target, so valid make -j4 build commands are reported as missing target -j4, while make -j build skips checking build. Parse options separately from targets so parallel-build examples are checked correctly.

Medium severity Resolve script paths against the current working directory

.github/​skills/​docs-sync-audit/​scripts/​docs_drift.py:455

A README containing cd tools followed by python check.py reports a missing script even when tools/check.py exists, because script checks ignore the tracked working directory. Resolve each script invocation against its execution directory at that point in the fenced block, including directory changes on the same line.

1 resolved since last review

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Human assessment is needed to weigh a critical release-baseline selection error and several validation and exit-status issues omitted from the published review.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Human assessment is needed to weigh omitted moderate findings involving false documentation alerts, unreliable batch error statuses, and incorrect release baselines.

No open issues 🎉

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟡 Changes recommended

Unresolved defects skip Makefile and workspace validation, produce false Markdown link findings, and cause batch scripts to corrupt arguments or report success for rejected input.

5 open issues

Review effort: Balanced

Comment thread .github/skills/batch-files/SKILL.md Resolved
vartaxe and others added 2 commits November 1, 2026 06:41
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

The nested SETLOCAL/ENDLOCAL transfers and delayed-expansion escaping require Windows cmd.exe validation with the CI-pinned Pester version, which was unavailable during this review.

No open issues 🎉

Review effort: Balanced

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.

🔵 Needs a closer look

Intricate CMD delayed-expansion and ENDLOCAL value transfers, plus Windows file-lock cleanup behavior, require maintainer validation in Windows CMD with the repository’s Pester version.

No open issues 🎉

Review effort: Balanced

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.

🟡 Changes recommended

The PowerShell tag filter accepts Unicode digits that cannot be converted to System.Version, causing discovery errors and inconsistent behavior with the Bash example.

1 open issue

Review effort: Balanced

Comment thread .github/skills/github-release/SKILL.md Resolved
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🔵 Needs a closer look

Human assessment is needed because an unpublished moderate-severity finding flags incorrect blank-separator output across three batch-file templates.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

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.

🔵 Needs a closer look

The CMD templates’ multi-phase expansion and nested SETLOCAL/ENDLOCAL transfers require cmd.exe runtime validation and the full Windows/Pester regression suite, which this review could not exercise.

No open issues 🎉

Review effort: Balanced

@cmendesvdl

Copy link
Copy Markdown
Collaborator

Validation follow-up for the runtime note. On PR head 0150d08106e469c64a2f8cf62151b8b8b1a6dd65, I ran the full Pester suite under Windows PowerShell 5.1.26100.9818 and Pester 6.2.0: 160 passed, 0 failed, 0 skipped, 0 not run. BatchExecutable.Tests.ps1, BatchLibrary.Tests.ps1, and BatchTask.Tests.ps1 execute the .cmd templates through $env:ComSpec /d /c, including nested SETLOCAL/ENDLOCAL and delayed-expansion handoffs. GitHub Actions run 41769412751 for this exact head also passed the standalone PowerShell regression harnesses, Pester, and manifest verification.

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.

🟡 Changes recommended

The padding routine silently succeeds without output when width is omitted or empty instead of returning the error used for other invalid widths.

1 open issue

Review effort: Balanced

Return the existing invalid-width error when padRight receives no width or an empty width.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

🟢 Approval recommended

No active review issues require resolution for this change.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

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.

🟢 Approval recommended

No active review issues are recorded for the automation skills, supporting templates and tests, or documentation updates.

No open issues 🎉

Review effort: Balanced

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.

🟡 Changes recommended

Argument forwarding through CALL re-expands percent expressions, potentially corrupting literal paths containing %USERNAME% despite delayed expansion being disabled.

1 open issue

Review effort: Balanced

REM Purpose: Describe what this script does
REM ============================================================

call :main %*
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.

3 participants