Fix #1619 skill findings: no-cluster/force, INPUT_PATH slashes, Step 1 gate, backup order, interpreter quoting - #3621
Conversation
Step 2 suggested a no cluster shortcut for a flat corpus and the number 479 shrink guard error suggested a forced rebuild, but neither flag was ever implemented: Step 4 called cluster() unconditionally and to_json() never received force=, so both were ghost flags the skill referenced but could not act on. Step 4 now builds a single "Full Corpus" community instead of calling cluster() when the no cluster flag was given, both to_json() calls take force=IS_FORCE, and Step 5 is skipped when there is only one placeholder community to not label. Applied to the core fragment and both hand maintained monoliths, with a new sanctioned diff predicate in gen.py's monolith round trip guard for the deliberate change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers the core render plus both monoliths: Usage lists both flags, Step 4 branches on the no cluster substitution instead of always clustering, both to_json calls receive the force substitution, and Step 5 documents skipping labeling when there is nothing to label. Updates the two existing literal string assertions that pinned the pre fix Step 5 to_json call verbatim. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A Windows path pasted in with backslashes splices a stray escape into the Python string literal INPUT_PATH is substituted into: a lone t becomes a tab and a lone U raises a syntax error, corrupting the block silently or loudly. One rule near the top of each host now tells the agent to substitute forward slashes instead, and the PowerShell Resolve Path call that saves the scan root is now quoted so a path containing a space survives too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Checks every host for the forward slash substitution rule and asserts the Windows Resolve Path call is quoted, not just the unquoted form being gone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ayushcodes10
changed the title
A failed install left the working interpreter variable pointing at one that still could not import graphify, and nothing checked for it: the step fell through silently, wrote that interpreter's path anyway, and every later step then failed with a cryptic command not found error far from the real cause. Step 1 now re checks the import right after the install attempt and stops with an actionable error naming both install commands instead of continuing. Applied to the shared posix and PowerShell install fragments and both hand maintained monoliths, with a matching sanctioned diff predicate in gen.py's monolith round trip guard. Verified by running the actual generated Step 1 bash block under a fake interpreter and PATH where the import always fails and pip always fails too: it now stops with the new error and exit code instead of writing a broken interpreter path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Asserts the failure message is present on every host and that it fires before the interpreter path is persisted for later steps to read. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ayushcodes10
changed the title
The old graph backup instruction appeared after both the merge block and the diff block that reads the backup, so an agent reading the runbook top to bottom never created the backup file before the merge ran. The diff block's presence check on that file then silently found nothing and skipped the diff on every single update, instead of only when a backup was genuinely unavailable. The instruction now appears right before the merge block it belongs with, in the shared update reference and both hand maintained monoliths, with a matching sanctioned diff predicate in gen.py's monolith round trip guard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Asserts the backup instruction precedes both the merge call and the diff block that reads the backup file, across the shared update reference and both monoliths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ayushcodes10
changed the title
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Graphify review — findings
Fixes four bugs in the /graphify skill runbooks: reorders the --update graph backup so it runs before the merge and diff that consume it, adds a hard-stop error in Step 1 when the interpreter still can't import graphify after retry instead of writing a broken path, and instructs every host to substitute INPUT_PATH with forward slashes (plus quotes the PowerShell Resolve-Path) so Windows paths don't corrupt the Python string literal. Wires up the previously-dangling --no-cluster and --force flags — Step 4 now builds a single "Full Corpus" community and skips Step 5 labeling under --no-cluster, and both to_json() calls pass force= through. The generator, fragments, expected fixtures, and tests are updated to match.
Worth a look
- INPUT_PATH substitution into Python string literals enables code injection —
graphify/skill-amp.md:56· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unquoted INPUT_PATH substitution in shell command allows command injection —
graphify/skill-copilot.md:113· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unescaped INPUT_PATH substitution enables code injection —
graphify/skill-kilo.md:56· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- --no-cluster skips the only graph.json label export —
graphify/skill-agents.md:431· Escalate · medium · 2 independent checks- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- --no-cluster Full Corpus label is never exported to graph.json —
graphify/skill-agents.md:451· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1522 functions depend on the 1522 functions this change touches.
Health — this change adds coupling hotspots:
- new:
render()— 17 callers, 5 callees - new:
audit_coverage()— 8 callers, 6 callees - new:
main()— 3 callers, 11 callees - new:
monolith_roundtrip()— 3 callers, 5 callees - new:
test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees
Verification — 1522 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: 1522 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
286 of 286 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/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— impact, changed-test, full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_external_call_guards.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safety- … and 236 more
non-code file(s) changed (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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 (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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.
· 5 more finding(s) on lines outside this diff (see the check run).
The saved interpreter path substitution names the interpreter to run and was spliced in unquoted on every one of its 100+ call sites, unlike the already quoted "$PYTHON" form Step 1 itself uses. An interpreter path containing a space, such as a venv under a user directory with a space in its name, word splits into multiple arguments and fails to exec. Every occurrence across the core fragment, both hand maintained monoliths, and five shared reference fragments is now quoted, plus the prose line describing the substitution pattern. The PowerShell translator's match constant is updated to the quoted form too, since it still recognizes the pattern by exact text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Asserts every host that references the interpreter substitution uses only the quoted form, with no unquoted survivor anywhere in its rendered output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test_devin_skill_file_uses_python_c_syntax pinned the pre fix unquoted substring, which the interpreter quoting fix legitimately changed. Updated to assert the quoted form instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ayushcodes10
changed the title
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Graphify review — findings
Quotes the /graphify skill's $(cat graphify-out/.graphify_python) interpreter substitution at every one of its 100+ call sites so an interpreter path containing a space no longer word-splits and fails to exec. Wires up the previously-referenced-but-unimplemented --no-cluster (builds a single "Full Corpus" community and skips labeling) and --force (passes force= through both to_json() calls) flags, hard-stops Step 1 with an actionable error when the interpreter still can't import graphify after retry, moves the pre-merge graph backup instruction ahead of the merge/diff block so --update diffs actually run, and directs every host to substitute INPUT_PATH with forward slashes so Windows backslashes don't corrupt the Python literal. Regenerates the per-platform skill artifacts, expected fixtures, and rationale entries to match.
Worth a look
- Unquoted INPUT_PATH substitution enables shell command injection —
graphify/skill-claw.md:113· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Raw INPUT_PATH substitution allows shell command injection —
graphify/skill-devin.md:68· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unescaped path substitution reaches double-quoted Python -c blocks —
graphify/skill-kilo.md:58· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- New to_json(force=...) call has no version/feature gate —
graphify/skill-kilo.md:96· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- --no-cluster label is not exported to graph.json —
graphify/skill-amp.md:453· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2214 functions depend on the 2214 functions this change touches.
Health — this change adds coupling hotspots:
- new:
render()— 18 callers, 5 callees - new:
audit_coverage()— 8 callers, 6 callees - new:
main()— 3 callers, 11 callees - new:
monolith_roundtrip()— 3 callers, 5 callees - new:
test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees
Verification — 2214 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: 2214 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
286 of 286 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/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— impact, changed-test, full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_external_call_guards.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safety- … and 236 more
non-code file(s) changed (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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 (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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.
· 5 more finding(s) on lines outside this diff (see the check run).
A reviewer found that a build with clustering skipped silently lost every node's community_name. Step 5 is the step that normally supplies real community labels, but it is skipped entirely when clustering was skipped, making Step 4's own write the only one for that path. Step 4 already computes the Full Corpus placeholder label just above this call, but never passed it through to to_json, so no node ended up with a community_name at all. Applied to the shared core fragment and both hand maintained monoliths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The to_json call itself was already covered by the existing Graphify-Labs#1392 predicate's generic match, but the new explanatory comment lines needed their own sanctioned entry or the round trip check treats them as unexplained drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Counts occurrences of the to_json call shape rather than a bare membership check, since Step 5's identical text alone would make a membership check pass even if Step 4's copy were still missing community_labels. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Good catch — fixed. A build with clustering skipped ( |
|
Verified the INPUT_PATH shell injection findings too — they're real (the Step 1 `echo "$(cd INPUT_PATH && pwd)"` line is unquoted, and bash still expands `$(...)`/backticks inside double quotes). This is pre-existing on pristine v8 though, not introduced by this PR, and it's in the shared `shell/posix.md`/`shell/powershell.md` fragments used by essentially every platform — a much broader, foundational fix than this PR's scope. Filed separately as #3642 with a suggested fix direction, rather than folding a wide, security sensitive rewrite into this PR. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Graphify review — findings
Wires up and enforces safe interpreter/path handling across the /graphify skill runbooks: every $(cat graphify-out/.graphify_python) call site is now quoted so venv paths with spaces exec correctly, Step 1 hard-fails with an actionable error when graphify still can't be imported after the retry instead of writing a broken path, and INPUT_PATH must be substituted with forward slashes to avoid corrupting the Python string literal on Windows. Reorders the --update runbook so the backup instruction precedes the merge/diff block that reads it, so the post-update diff actually runs. Implements the previously-dangling --no-cluster and --force flags: Step 4 builds a single "Full Corpus" community (and passes that label through on write) instead of clustering, both to_json() calls honor force=, and Step 5 skips labeling when there's nothing to label.
Worth a look
- Unescaped path substitution enables Python code injection —
graphify/skill-amp.md:56· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unquoted INPUT_PATH in shell command enables command injection —
graphify/skill-claw.md:115· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Raw INPUT_PATH substitution can execute shell commands —
graphify/skill-copilot.md:56· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Raw INPUT_PATH substitution into double-quoted python -c blocks enables command injection —
graphify/skill-devin.md:67· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Unescaped INPUT_PATH substitution permits Python code injection —
graphify/skill-droid.md:56· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2216 functions depend on the 2216 functions this change touches.
Health — this change adds coupling hotspots:
- new:
render()— 19 callers, 5 callees - new:
audit_coverage()— 8 callers, 6 callees - new:
main()— 3 callers, 11 callees - new:
monolith_roundtrip()— 3 callers, 5 callees - new:
test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees
Verification — 2216 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: 2216 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
286 of 286 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/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— impact, changed-test, full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_external_call_guards.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safety- … and 236 more
non-code file(s) changed (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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 (
CHANGELOG.md,graphify/skill-agents.md,graphify/skill-aider.md,graphify/skill-amp.md,graphify/skill-claw.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.
· 5 more finding(s) on lines outside this diff (see the check run).
1 participant
Summary
Fixes five findings from #1619's Windows skill review:
--no-cluster(Step 2's own suggestion) and--force(the shrink-guard error's own suggestion) were referenced but never implemented in the generated skill's Python. Both are realgraphifyCLI flags already (seecli.py), so this wires the skill's inline Python through to the same underlying support.INPUT_PATHwith backslashes splices a stray escape into the Python string literal it lands in (\tbecomes a tab,\Uraises aSyntaxError); the PowerShellResolve-Pathcall that saves the scan root was also unquoted.--updaterunbook's "save the old graph" instruction appeared AFTER both the merge block and the diff block that reads the backup, so an agent following it top to bottom never created the backup before the merge ran, and the post-update diff silently no-opped on every single update.$(cat graphify-out/.graphify_python)) was spliced in unquoted at 100+ call sites, unlike the already-quoted"$PYTHON"form Step 1 itself uses — an interpreter path containing a space word-splits into multiple arguments and fails to exec.(A2/A1 from the same issue were already fixed; C3 is fixed by the already-open #2782, cross-referenced on the issue. B2/B3/C5/D remain open — B3 turned out to already be resolved by the current architecture, since the whole Windows core is now translated to consistent PowerShell rather than mixing shells.)
Fix
IS_NO_CLUSTERandIS_FORCE, follow the pattern already established forIS_DIRECTED. Step 4 branches to a single "Full Corpus" community when--no-clusterwas given instead of callingcluster(G); bothto_json()calls passforce=IS_FORCE; Step 5 documents skipping labeling when there's nothing to label.INPUT_PATHwith forward slashes on Windows; the PowerShellResolve-Pathcall is now quoted.--updaterunbook's backup instruction now appears right before the merge block it belongs with.aider.md,devin.md), with new sanctioned-diff predicates ingen.py's monolith round-trip guard for each deliberate change.--no-cluster/--forceto the## Usageblock.Test plan
tests/test_skillgen.py:test_no_cluster_and_force_flags_are_wired_through,test_input_path_forward_slash_guidance_is_present,test_step1_gates_on_a_still_failed_install,test_update_backup_instruction_precedes_the_merge_it_backs_up,test_interpreter_cat_substitution_is_quoted_everywhere— each covering the core render,skill-windows.md, and both monoliths.tests/test_skillgen.py,tests/test_community_labels_skill.py, andtests/test_devin.pythat pinned pre-fix output verbatim.python3 -m tools.skillgen --check/--audit-coverage/--monolith-roundtrip/--schema-singleton— all OK.--no-cluster/--forceend to end against a real extraction, the actual generated Step 1 bash block under a fake failing interpreter/PATH, and a quoted interpreter invocation directly in bash.python3 -m pytest -q— 5626 passed, 68 skipped, no regressions.🤖 Generated with Claude Code