Skip to content

fix: state shell requirements with a Windows path across 8 plugins#331

Merged
kyle-sexton merged 2 commits into
mainfrom
feat/cross-platform-declarations
Jul 18, 2026
Merged

fix: state shell requirements with a Windows path across 8 plugins#331
kyle-sexton merged 2 commits into
mainfrom
feat/cross-platform-declarations

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Tranche C of the prerequisite-visibility wave: every dim-10 cross-platform FAIL from the conformance audit — undeclared shell/coreutils assumptions get a stated requirement with the Git Bash Windows path, and the two structural Windows hazards get real fixes.

Per-plugin changes

Plugin Version Change
mcp-tools 0.2.1 README Requirements section added (Bash + coreutils, jq, Git Bash path). CHANGELOG created.
ai-briefing 0.5.1 README states the POSIX-shell requirement of setup --with-build-deps (script's platform gate already accepts MINGW/MSYS/CYGWIN — the doc prong was the gap).
education 0.3.1 README corrected from "none beyond Claude Code": declares sha256sum/shasum, realpath, tr, sed with the Git Bash path (bundles all of them). CHANGELOG created.
prototype 0.2.4 README declares the detection-script Bash requirement and documents the no-Bash degrade honestly (pre-computed context echoes "none detected"; the skills read the host project directly — verified against skills/logic/SKILL.md, which never branches on the detection output).
code-tidying 0.4.3 README declares Bash 4+ (mapfile, case-conversion expansions) with the Git Bash path; notes the scripts' existing CRLF/drive-letter handling.
knowledge 0.7.1 README declares the Git Bash shell mechanics (book-distill hashes with sha256sum on every run) and the EPUB branch's unzip requirement (not bundled with Git Bash); PDF-only use needs neither.
event-storming 0.5.3 Simulation teardown phrased shell-agnostically at both sites (rm -rf on POSIX/Git Bash, Remove-Item -Recurse -Force on PowerShell) instead of unconditional rm -rf.
repo-hygiene 0.2.1 clean's PreToolUse destructive-guard hook moves from a bare-.sh command to the interpreter-named exec form (command: "bash", args: […]) — the doctrine-prescribed Windows-safe spawn shape, matching disk-hygiene's existing command: "python" precedent.

Verification

Related

Closes #317 (together with #328 and #329, which carry tranches A and B).

🤖 Generated with Claude Code

Cross-platform declaration wave (dim 10): mcp-tools, education (README
corrected from "none beyond Claude Code"), ai-briefing (setup
--with-build-deps POSIX-shell requirement), prototype (detection-script
Bash requirement + documented no-Bash degrade), code-tidying (Bash 4+),
knowledge (Git Bash sha256sum for book-distill; unzip for the EPUB
branch) now declare their shell/coreutils mechanics with the Git Bash
Windows path. event-storming's simulation teardown is phrased
shell-agnostically (rm -rf / Remove-Item) at both sites. repo-hygiene's
destructive-guard hook moves to the interpreter-named exec form
(command: bash + args) — the doctrine-prescribed Windows-safe spawn
shape. CHANGELOGs created for mcp-tools and education.

Part of #317 (tranche C).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8dfbb16b08

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/event-storming/skills/simulation/reference/agentic-simulation.md Outdated
Codex review P2: the PowerShell example passed {session_dir} unquoted,
so a temp path containing a space would mis-parse. Both teardown sites
now quote the path and the PowerShell form uses -LiteralPath.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton merged commit b2a67b8 into main Jul 18, 2026
19 of 20 checks passed
@kyle-sexton
kyle-sexton deleted the feat/cross-platform-declarations branch July 18, 2026 07:05
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 34s —— View job


PR Review — fix: state shell requirements with a Windows path across 8 plugins

  • Gather context and understand the request
  • Review PR diff for correctness, security, and guideline alignment
  • Read changed files in depth
  • Post final review findings

Summary

This tranche addresses cross-platform (dim-10) conformance fails by documenting shell/coreutils prerequisites with Windows Git Bash paths across 8 plugins and making two structural fixes. The documentation changes are accurate and consistent. One confirmed issue slipped through the merge.


Issue: PowerShell Remove-Item path unquoted — event-storming

File: plugins/event-storming/skills/simulation/reference/agentic-simulation.md

The PR adds PowerShell cleanup alternatives at two teardown sites, but {session_dir} is left unquoted in both:

Remove-Item -Recurse -Force {session_dir}

Failure scenario: On native Windows, %TEMP% commonly resolves to a path containing spaces (e.g. C:\Users\Jane Doe\AppData\Local\Temp). PowerShell's parser splits on whitespace, so Remove-Item -Recurse -Force C:\Users\Jane Doe\AppData\... treats Doe\AppData\... as a separate positional argument, causing a PathNotFound error. Teardown silently fails; temp session directories accumulate.

The Codex reviewer caught this at 06:06 UTC and @kyle-sexton acknowledged it: "Fixed in 623b728: both teardown sites now quote the interpolated path, and the PowerShell form uses Remove-Item -LiteralPath "..." -Recurse -Force." However, commit 623b72854e is not present in the merged HEAD (8dfbb16b). The merged file still has the unquoted form at both sites (lines 338 and 652).

Required fix (at both sites):

Remove-Item -LiteralPath "{session_dir}" -Recurse -Force

Using -LiteralPath rather than the positional parameter also defends against paths with [ ] characters that PowerShell's default parameter treats as glob wildcards.

Fix this →


Findings by plugin

repo-hygiene — hook form change (SKILL.md)

-  command: "\"${CLAUDE_PLUGIN_ROOT}\"/skills/clean/scripts/destructive-guard.sh"
+  command: "bash"
+  args: ["${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/destructive-guard.sh"]

This is the correct Windows-safe spawn shape. The old form required the OS to execute shell scripts via shebang, which native Windows can't do. The new form explicitly invokes bash, matching the command: "python" precedent from disk-hygiene. Correct and clean.

ai-briefing — README

Documents the existing MINGW*/MSYS*/CYGWIN* platform gate in the install step that already accepted Git Bash environments. The doc prong was the only gap; this closes it accurately.

education — README

Replaces the inaccurate "none beyond Claude Code" with a concrete list: sha256sum/shasum, realpath, tr, sed, all bundled with Git Bash. Accurate and actionable.

prototype — README

Honest about graceful degrade: without Bash the detection script reports "none detected" and skills read the host project directly. The degrade behaviour is documented so Windows users know what to expect.

code-tidying — README

Documents Bash 4+ (mapfile, case-conversion expansions) requirement and notes that existing scripts already handle CRLF/drive-letter paths — a useful confirmation for Windows users.

knowledge — README

Two requirements correctly separated: Bash+coreutils for hash mechanics (bundled with Git Bash), and unzip for the EPUB branch (not bundled — correctly flagged as a separate install). PDF-only use is explicitly carved out.

mcp-tools — README

Requirements section added: Bash+coreutils for inline mechanics, jq for JSON (separate install on Git Bash). Link to jqlang.org/download provided.

event-storming — teardown ⚠️ See finding above.


Minor observations (no action required)

  • unzip degrade for knowledge/EPUB: unlike prototype (which documents a no-Bash fallback path), the knowledge EPUB branch has no degrade path documented — it will simply fail if unzip is absent. This is acceptable given EPUB is a clearly optional branch, but a brief note ("extract the EPUB manually with 7-Zip or another archive tool, then run the skill on the extracted directory") would improve the UX.
  • plugin.json keyword reformatting: multiple plugin.json files have their keywords arrays reformatted from inline to multi-line. These are cosmetic but harmless and improve diff readability for future edits.
  • Version bumps: all 8 plugins get appropriate patch-level bumps with matching CHANGELOG entries. The CHANGELOG format (Keep a Changelog 1.1.0 + semver) is consistent across all.

Verdict

The structural fix (repo-hygiene hook form) and the 7 documentation additions are correct and well-scoped. The one issue — unquoted {session_dir} in the event-storming PowerShell teardown — is a real failure path on Windows and should be patched on main since the intended fix didn't make it into the merge.

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 45s —— View job


  • Gather context and understand the request
  • Verify the fix in commit 623b72854e for the unquoted PowerShell path
  • Report findings

Verification — PowerShell Remove-Item path fix

Commit 623b72854e is confirmed to contain the fix at both teardown sites in plugins/event-storming/skills/simulation/reference/agentic-simulation.md:

Line 338:

Remove-Item -LiteralPath "{session_dir}" -Recurse -Force

Line 652:

Remove-Item -LiteralPath "{session_dir}" -Recurse -Force

Both sites now use -LiteralPath with the path quoted, which handles:

  • Paths containing spaces (e.g. C:\Users\Jane Doe\...) ✅
  • Paths containing [ ] characters that PowerShell's default positional parameter treats as glob wildcards ✅

The intended fix from @kyle-sexton's commit 623b72854e is present and correct. The earlier review comment noting the fix "was not included in the merge" reflected a race between the PR merge and the push of 623b72854e; the current HEAD of this branch has the complete fix.

No further action required for this issue.

kyle-sexton added a commit that referenced this pull request Jul 19, 2026
…nostic skills (#492)

## Summary

The `verification` plugin's `confirm` and `measure` skills are sold as
ecosystem-agnostic, but their illustrative examples leaned
.NET/C#/PowerShell, and two "how to check" cells assumed a Unix shell
(`wc -l`) against the cross-platform "never assume Bash" contract. A
non-.NET consumer was handed a stack-specific illustration as the
universal path. This patch neutralizes the examples while keeping the
guidance concrete and useful.

## Fix

- `skills/confirm/context/fix.md` — reproduction-test example now uses a
generic descriptive name (`returns_null_when_user_is_deleted`, "name it
per your framework's convention") and a generic failure ("the reported
failure (e.g. a null-dereference crash)") instead of
`UserService_ShouldReturnNull_WhenUserIsDeleted` /
`NullReferenceException`.
- `skills/measure/context/metrics.md` — coupling row counts
"import/dependency declarations (`import`/`require`/`using`, package or
project references)" instead of `using`/`ProjectReference`; coverage row
points to "your test runner's output" instead of `dotnet test`; both `wc
-l` sites are now stated shell-neutrally (`wc -l` on POSIX/Git Bash,
`Measure-Object -Line` in PowerShell), mirroring the both-forms pattern
established in #331.
- `skills/measure/context/performance.md` — metric "how to measure"
cells point to "your test runner", "your build tool", "your platform's
profiler", and "your benchmark harness", with the shell timer given for
both shells (`time` on POSIX/Git Bash, `Measure-Command` in PowerShell)
instead of `dotnet test`/`dotnet-counters`/BenchmarkDotNet as the only
path.
- The named marketplace-plugin evidence pointers (`dotnet-*:*`) at the
bottom of both `measure` files are unchanged — those are real named
plugins used as evidence sources, not universal-path illustrations, and
#424 deliberately did not flag them.
- `verification` bumped `0.2.2` → `0.2.3` (docs = patch) with a
CHANGELOG entry.

## Verification

Gates run in this Windows environment against the changed markdown; all
clean, nothing suppressed:

- `markdownlint-cli2 v0.23.0 (markdownlint v0.41.0)` (repo-pinned) on
the four changed `.md` files — `Summary: 0 error(s)`.
- `typos-cli 1.44.0` on the four changed `.md` files — exit 0, no
output.
- `editorconfig-checker v3.8.0` on the changed `.md` files plus
`plugin.json` — exit 0, no findings.
- `plugin.json` re-parsed as valid JSON; version reads `0.2.3`.

## Related

Closes #424. Sibling stack-bias issues in the same work-readiness sweep,
same pattern: #430, #429, #428 (all `testing` plugin).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Conformance wave: prerequisite visibility + cross-platform declarations (incl. shared hook-utils fix)

1 participant