Skip to content

fix(skillgen): translate Windows reference docs to PowerShell - #4987

Open
wannT-1 wants to merge 1 commit into
Graphify-Labs:v8from
wannT-1:cursor/windows-ref-powershell-ab33
Open

wannT-1 wants to merge 1 commit into
Graphify-Labs:v8from
wannT-1:cursor/windows-ref-powershell-ab33

Conversation

@wannT-1

@wannT-1 wannT-1 commented Nov 4, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes #3600.

Windows skill references were copied from the shared POSIX fragments even when shell = "powershell". The strict translator already rewrote the core skill body, so graphify/skills/windows/references/*.md still shipped bash fences: $(cat ...), export, if [ ! -f ], cp / rm -f, and (where a fragment uses it) mktemp /tmp/graphify_*.XXXXXX.

This runs _core_to_powershell over references/*.md for powershell-shell hosts and extends that translator with the reference patterns, still failing the render on any bash line it does not recognize.

mktemp capture:

  • VAR=$(mktemp /tmp/graphify_*.XXXXXX) becomes $VAR = [System.IO.Path]::GetTempFileName()
  • a bare mktemp /tmp/graphify_*.XXXXXX (the form that prints the path) becomes the same call with no assignment

GetTempFileName() creates an empty temp file and returns its full path, so the path is captured the way $(mktemp ...) was. The /tmp/graphify_* template is not passed through; any other mktemp shape still fails the render.

Other reference POSIX that the shared fragments actually contain is translated the same way ($(cat .graphify_python) -m, "$(cat .graphify_root)", export, if [ ! -f ], VAR=$(graphify ...), bash backslash continuations, and ~/ on those commands). POSIX hosts are unchanged. The core Windows skill body is unchanged.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

The powershell render stays strict: an unrecognized bash line in a Windows reference raises at render time, and banned bash tokens cannot survive into the artifact.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

How was this tested?

python3 -m pytest tests/test_skillgen.py -q --tb=short
python3 -m tools.skillgen --check
python3 -m ruff check tools/skillgen/gen.py tests/test_skillgen.py

76 skillgen tests passed. tools.skillgen --check matched all 134 artifacts. Ruff reported no issues.

Graphify-specific checklist

  • I updated generated skill artifacts when changing their source fragments.
  • I confirmed that AST/structural extraction remains deterministic.
  • I reviewed changes for security implications.
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

@wannT-1
wannT-1 requested a review from safishamsi as a code owner November 4, 2026 01:28
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Thanks for the pull request, @wannT-1. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

@graphify-labs graphify-labs 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.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.

Not checked: tests were not run; formal verification proved 0 of 5 changed function(s) (3 sampled, not proven, 2 not verified).

Formal verification. PR-changed functions: 3/5 verified (0 proven, 3 may-equivalent, 0 distinguished) · 2 not verified (2 vacuous).

Not verified on this run: \_translate\_bash\_block (vacuous: never exercised), render (vacuous: never exercised).


Graphify review — findings

Adds PowerShell-native versions of the Windows skill's reference docs, replacing the bash blocks with powershell fences generated by the skillgen translator. Inline Python now runs through a single-quoted here-string piped into the interpreter path read from .graphify_python, line continuations become backticks, and ~ becomes $HOME; the tests assert no bash leftovers, no BOM on written marker files, and that POSIX hosts keep their bash invocations. The piped-here-string and double-quoted interpolation forms are where this can go wrong: query, path and explain arguments are expandable strings, non-ASCII text can be mangled on its way to Python, and --watch fails when .graphify_root is empty.

Worth a look

  • PowerShell query/path/explain examples use injectable expandable strings — graphify/skills/windows/references/query.md:64 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
  • PowerShell transcription env var examples allow quote-breakout injection — graphify/skills/windows/references/transcribe.md:26 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
  • Piped here-string mangles non-ASCII in Windows PowerShell 5.1 — graphify/skills/windows/references/query.md:19 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review

Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 405 functions depend on the 383 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: render() — 17 callers, 7 callees
  • worse: _translate_bash_block() — 3 callers, 12 callees

Verification — 405 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 405 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

518 of 518 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/pi.test.ts — full-run-safety
  • tests/test_4874_jvm_resolution.py — full-run-safety
  • tests/test_4909_reference_binding_scale.py — full-run-safety
  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_defines.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_affected_overrides.py — full-run-safety
  • tests/test_affected_path_file_node.py — full-run-safety
  • tests/test_affected_seed_reference_forms.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_alias_folding.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — full-run-safety
  • tests/test_astro_import_ids.py — full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_backend_opt_in.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_benchmark_real_query.py — full-run-safety
  • tests/test_binary_media_words.py — full-run-safety
  • tests/test_blade_extractor.py — full-run-safety
  • tests/test_build.py — full-run-safety
  • tests/test_build_located_semantic_identity.py — full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_ghost_fold_4955.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — full-run-safety
  • tests/test_c_include_resolution.py — full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_cache_long_paths.py — full-run-safety
  • tests/test_cache_stale_import_target.py — full-run-safety
  • tests/test_cache_windows_case_4925.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_cargo_missing_manifest.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_only_decl_ids_4904.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — full-run-safety
  • … and 468 more

non-code file(s) changed (graphify/skills/windows/references/add-watch.md, graphify/skills/windows/references/exports.md, graphify/skills/windows/references/github-and-merge.md, graphify/skills/windows/references/hooks.md, graphify/skills/windows/references/query.md …) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (graphify/skills/windows/references/add-watch.md, graphify/skills/windows/references/exports.md, graphify/skills/windows/references/github-and-merge.md, graphify/skills/windows/references/hooks.md, graphify/skills/windows/references/query.md …) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

No difference found (not proven): No behavior difference found in \_core\_to\_powershell (not a proof).

The verifier ran both versions of \_core\_to\_powershell on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify \_translate\_bash\_block.

The verifier did not have enough to check \_translate\_bash\_block, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

No difference found (not proven): No behavior difference found in \_translate\_prose\_line (not a proof).

The verifier ran both versions of \_translate\_prose\_line on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

No difference found (not proven): No behavior difference found in \_unescape\_bash\_dq (not a proof).

The verifier ran both versions of \_unescape\_bash\_dq on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

Could not verify: Could not verify render.

The verifier did not have enough to check render, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 3 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly ValueError — names the real obstacle, not a sampling gap)

· 1 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).

Comment thread tools/skillgen/gen.py
return line[: len(line) - len(line.lstrip())]


def _translate_bash_block(lines: list[str]) -> list[str]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Health regression — _translate_bash_block()

fans out to 12 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

The strict POSIX-to-PowerShell pass only ran on the core skill body, so
windows references still shipped bash fences. Run that translator over
references when shell is powershell, including a mktemp path-capture rule,
and regenerate the Windows artifacts.

Fixes Graphify-Labs#3600

Co-authored-by: wannT-1 <wannT-1@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/windows-ref-powershell-ab33 branch from bb67e93 to bae295a Compare November 9, 2026 05:17

This branch has not been deployed

No deployments
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.

Windows skill reference docs assume mktemp/POSIX paths despite shell = powershell

1 participant