fix(paths): resolve Windows mkstemp concurrency deadlock (#3646) - #3663
nikhilsaxena04 wants to merge 1 commit into
Conversation
…bs#3646) - Wrap tempfile.mkstemp in a bounded retry loop (5 attempts, 100ms backoff) to gracefully handle transient PermissionErrors caused by Windows antivirus scanners. - Avoids infinite hang that occurred when mkstemp hit an EACCES collision on Windows. - Add regression tests to test_paths.py
![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 1 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_id\_prefixes changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_id\_prefixes behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"source\_file":"'h\\u00e9llo w\\u00f6rld'"\}, the old code produced \{'héllo\_wörld'\} but the new code produces \{'h\_llo\_w\_rld'\}. Paste that input straight into a regression test.
Graphify review — findings
Retries mkstemp in _atomic_replace up to 5 times with a 0.1s sleep between attempts when it hits a PermissionError, so transient locks from antivirus or indexing services no longer fail the atomic write; after exhausting the retries it raises a RuntimeError naming the target directory and chaining the last error. Adds tests covering both the eventual-success and permanent-failure paths.
Worth a look
- Persistent mkstemp PermissionError is remapped to RuntimeError —
graphify/paths.py:63· 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 — 1873 functions depend on the 45 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 563 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
build_from_json()— 199 callers, 19 callees - new:
build_merge()— 65 callers, 14 callees - new:
to_obsidian()— 38 callers, 14 callees - new:
save_manifest()— 40 callers, 11 callees - new:
to_json()— 56 callers, 7 callees - new:
to_wiki()— 41 callers, 7 callees - …and 41 more — each is listed as a finding
Verification — 1873 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: 959 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
79 of 262 test file(s) selected (30%) via static blast radius.
tests/test_analyze.py— impacttests/test_atomic_canvas_export.py— impacttests/test_atomic_writes.py— impacttests/test_benchmark.py— impacttests/test_benchmark_raw_graph.py— impacttests/test_build.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_callflow_html.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_cli_export.py— impacttests/test_cluster.py— impacttests/test_community_labels_skill.py— impacttests/test_confidence.py— impacttests/test_corrupt_graph_json.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_evidence_binding.py— impacttests/test_export.py— impacttests/test_export_control_characters.py— impacttests/test_export_path_length.py— impacttests/test_extract.py— impacttests/test_extract_cli.py— impacttests/test_falkordb_integration.py— impacttests/test_file_label_disambiguation.py— impacttests/test_global_graph.py— impacttests/test_go_qualified_resolution.py— impacttests/test_god_nodes_exclude_hubs.py— impacttests/test_hook_out_of_project_paths.py— impacttests/test_hook_strict.py— impacttests/test_hyperedge_member_shapes.py— impacttests/test_hyperedge_roundtrip.py— impacttests/test_hypergraph.py— impacttests/test_import_self_loops.py— impacttests/test_incremental_mtime_collision.py— impacttests/test_java_type_resolution.py— impacttests/test_labeling.py— impacttests/test_languages.py— impacttests/test_manifest_ingest.py— impacttests/test_multigraph_diagnostics.py— impacttests/test_no_dedup_flag.py— impacttests/test_non_string_node_ids.py— impacttests/test_obsidian_dangling_member.py— impacttests/test_obsidian_filename_cap.py— impacttests/test_obsidian_unicode_tags.py— impact- … and 29 more
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
Behavior changes: \_id\_prefixes changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_id\_prefixes behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"source\_file":"'h\\u00e9llo w\\u00f6rld'"\}, the old code produced \{'héllo\_wörld'\} but the new code produces \{'h\_llo\_w\_rld'\}. Paste that input straight into a regression test.
Could not verify: Could not verify build.
The verifier did not have enough to check build, 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 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build\_from\_json.
The verifier did not have enough to check build\_from\_json, 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 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify build\_merge.
The verifier did not have enough to check build\_merge, 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: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify prefix\_graph\_for\_global.
The verifier did not have enough to check prefix\_graph\_for\_global, 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 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous
No difference found (not proven): No behavior difference found in \_flush\_stat\_index (not a proof).
The verifier ran both versions of \_flush\_stat\_index 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 save\_cached.
The verifier did not have enough to check save\_cached, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify save\_semantic\_cache.
The verifier did not have enough to check save\_semantic\_cache, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, 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 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly IndexError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in cohesion\_score (not a proof).
The verifier ran both versions of cohesion\_score 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 \_native\_leiden (not a proof).
The verifier ran both versions of \_native\_leiden 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 link\_cross\_repo\_member\_calls (not a proof).
The verifier ran both versions of link\_cross\_repo\_member\_calls 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 deduplicate\_entities.
The verifier did not have enough to check deduplicate\_entities, 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 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_llm\_tiebreak.
The verifier did not have enough to check \_llm\_tiebreak, 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 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify detect.
The verifier did not have enough to check detect, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_is\_noise\_dir.
The verifier did not have enough to check \_is\_noise\_dir, 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: parameter `parent` is annotated `'Path | None'` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_is\_prose\_note.
The verifier did not have enough to check \_is\_prose\_note, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify to\_graphml.
The verifier did not have enough to check to\_graphml, 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 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, 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: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_parallel.
The verifier did not have enough to check \_extract\_parallel, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_import\_php.
The verifier did not have enough to check \_import\_php, 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 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify extract\_bash.
The verifier did not have enough to check extract\_bash, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify extract\_elixir.
The verifier did not have enough to check extract\_elixir, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_js\_extra\_walk.
The verifier did not have enough to check \_js\_extra\_walk, 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 200 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
· 49 more finding(s) on lines outside this diff (see the check run).
1 participant
Hi! This PR fixes the infinite hang reported in #3646 where
graphify updatestalls indefinitely on Windows during manifest saves.The Fix:
tempfile.mkstempinside_atomic_replacein a bounded retry loop (5 attempts, 100ms backoff).PermissionErrors on Windows (e.g., when an antivirus scan briefly locks the directory) instead of triggering Python's infinite internal retry loop.RuntimeErrorrather than silently hanging the CLI forever.test_paths.pywith mockedPermissionErrors.Tested locally and the full suite passes!