fix(install): write .graphify_version atomically via os.replace - #3303
fix(install): write .graphify_version atomically via os.replace#3303drmikecrypto wants to merge 1 commit into
Conversation
Match the SKILL.md / references/ crash-safety pattern so an interrupted install cannot leave a truncated stamp, and replace managed symlinks instead of writing through them. Fixes Graphify-Labs#3286
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 3 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Writes the .graphify_version stamp atomically via _write_version_stamp, which stages a temp file in the target directory and os.replaces it into place, replacing a managed symlink rather than following it and cleaning up the temp file on failure so an existing stamp survives a mid-write error. Routes both the main skill install (_copy_skill_file) and the VS Code path (vscode_install) through it instead of a direct write_text. Adds regression tests covering the happy path, temp cleanup with the old stamp preserved on os.replace failure, and symlink replacement.
Worth a look
- Fixed temp filename makes concurrent version stamp writes race —
graphify/install.py:43· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Version stamp temp file name is not safe for concurrent installs —
graphify/install.py:44· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Fixed temp filename follows attacker-controlled symlink —
graphify/install.py:45· 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 — 409 functions depend on the 146 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 123 callees - new:
codebuddy_install()— 20 callers, 5 callees - new:
claude_install()— 19 callers, 4 callees - new:
gemini_install()— 10 callers, 7 callees - new:
claude_uninstall()— 17 callers, 4 callees - new:
_project_uninstall()— 5 callers, 13 callees - new:
dispatch_install_cli()— 2 callers, 31 callees - new:
_copy_skill_file()— 12 callers, 5 callees - …and 13 more — each is listed as a finding
Verification — 409 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: 265 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_copy\_skill\_file.
The verifier did not have enough to check \_copy\_skill\_file, 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: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly KeyError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify vscode\_install.
The verifier did not have enough to check vscode\_install, 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: not verifiable: all 29 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)
· 21 more finding(s) on lines outside this diff (see the check run).
Match the SKILL.md / references/ crash-safety pattern so an interrupted install cannot leave a truncated stamp, and replace managed symlinks instead of writing through them.
Fixes #3286