[grloper]

Fixes for problems found while using v1.0.0 for real on Windows 11 and on a Unity-style C# codebase. One commit per fix plus one review-follow-up commit for Unity/.gitignore and one for Unity callback interfaces, then CHANGELOG/baseline. Nothing is merged, tagged, released or published; the crate version stays 1.0.0.

Per bug

1. Windows: plumb doctor reports every provider as missing

• Failing tests (written first): crates/core/tests/providers_exe_ext.rs: windows_extensions_find_exe_and_cmd_programs, windows_extension_order_follows_the_list (Linux, injected extension list) and real_windows_detection_finds_cargo_exe (cfg(windows), real PATHEXT).
  Before the fix, on Linux and natively on Windows: Detected { id: "cargo-check", ..., available: false, path: None, ... }.
• Root cause: find_program joined the bare catalog name (cargo, tsc) and checked is_file(). On Windows the files are cargo.exe / tsc.cmd, so nothing was ever found.
• Fix: executable_extensions() reads PATHEXT on Windows (and returns nothing elsewhere). Only .com/.exe/.bat/.cmd are kept, because those are what std::process::Command can start; .js/.vbs need a host. Lookup order is directory first, then extension, as in cmd.exe. A bare extensionless file is not accepted when extensions apply (npm puts an sh shim next to tsc.cmd). Unix keeps the exact-name + execute-bit lookup (no_extension_list_keeps_exact_name_lookup, unix_default_detection_is_exact_name_only, platform_extension_list_matches_the_platform). detect_with_exts is the test seam. The same lookup is used by enrich and by rule-engine detection.
• Proof: cargo test -p plumbgraph-core --test providers_exe_ext passes 7/7 on Linux and natively on Windows. A real Windows run of the built binary, plumb doctor C:\...\Wraith, now shows cargo-check installed C:\ProgramData\chocolatey\bin\cargo.exe and ruff installed ...\Scripts\ruff.exe; v1.0.0 showed both as missing.

2. plumb init prints an invalid TOML Codex snippet on Windows

• Failing tests: crates/core/tests/init_snippet.rs: codex_snippet_is_valid_toml_for_a_windows_verbatim_path, codex_snippet_maps_verbatim_unc_to_a_plain_unc_path, codex_snippet_round_trips_awkward_paths. They parse the snippet with the toml crate.
  Before: TOML parse error at line 4, column 31 ... missing escaped value, expected b, e, f, n, r, \, ", x, u, U on "\\?\C:\Users\...".
• Root cause: root.display() was interpolated into a TOML basic string. On Windows canonicalize() returns \\?\C:\..., and \U, \o, ... are escape sequences there.
• Fix: strip_verbatim (\\?\C:\ → C:\, \\?\UNC\srv\share → \\srv\share; \\?\Volume{..} and device paths are left alone) plus toml_string: a literal string 'C:\...' when possible, otherwise a basic string with ", \ and control characters escaped (round trip tested with ', ", tab, newline, DEL, BEL, non-ASCII). Paths in the init report use the plain form too.
• Proof: cargo test -p plumbgraph-core --test init_snippet passes 6/6 on Linux and on Windows. On Windows init_prints_a_parseable_codex_snippet_without_a_verbatim_prefix goes through a real canonicalize(). I also piped the snippet printed by the real Windows binary through Python's tomllib: the branch prints parsed OK, args = ['mcp', '--root', 'C:\\Users\\ofekv\\Desktop\\repos\\Wraith'], while v1.0.0 gives TOML ERROR: Unescaped '\' in a string (at line 3, column 32).

3. Dead-code false positives for Unity C#

• Failing tests: crates/core/tests/unity_entry_points.rs with the golden fixture crates/core/tests/fixtures/unity (Assets/, Server/). Before the fix it failed with false positive Player.Awake, false positive BuildTools.Bake, false positive GameSettings.OnEnable, and Player.Jump was not low. Pre-fix default output: 47 findings, every lifecycle method at medium 0.71; after: 13 on the same fixture before the review follow-ups added more fixture files.
  Honest note: my first version of these tests used Game.Player.Update qualified names. C# qnames do not include the namespace, so three negative tests passed vacuously. I caught this in the first run. symbols() now asserts a positive control (Player.NeverCalled is reported), so an absence check cannot pass on a naming mismatch.

• Root cause: Unity calls these methods by name, so the tier-0 graph has no caller, and only override/Main/constructors were C# entry points.

• Fix:

  • Messages (UNITY_MESSAGES: Awake, Start, Update, LateUpdate, FixedUpdate, OnEnable/Disable/Destroy, OnCollision*/OnTrigger* incl. 2D, OnGUI, OnValidate, Reset, rendering/mouse/animation/editor callbacks) are entry points for non-static methods of classes that:
    • derive directly from MonoBehaviour, ScriptableObject, NetworkBehaviour, StateMachineBehaviour or UIBehaviour; or
    • derive from Editor/EditorWindow in a Unity file; or
    • live under Assets/ with a first base type that is a class, not an I-prefixed interface (indirect Enemy : Character : MonoBehaviour).
  • Attributes in a Unity file: [RuntimeInitializeOnLoadMethod], [InitializeOnLoadMethod], [InitializeOnLoad], [MenuItem], [ContextMenu], [DidReloadScripts], [PostProcessBuild], [PostProcessScene], [OnOpenAsset], with or without the Attribute suffix.
  • Callback interfaces in a Unity file: ISerializationCallbackReceiver, IHasCustomMenu, UI EventSystem handlers. These apply to any implementing type.
  • "Unity file" means: under Assets/ or Packages/, or code (an AST check; comments and strings don't count) with a UnityEngine/UnityEditor using or qualified name.
  • Public methods of Unity classes keep their finding, with a 0.25 penalty and the fp_risk "may be bound to a UnityEvent (e.g. Button.onClick), an Animation Event or SendMessage in a scene, prefab or animation clip, which are not indexed". This goes through a new SymbolFact.framework_risk, stored in a new column; schema 3 / extractor 8, so old indexes rebuild once (existing mechanism).
  • [SerializeField] fields: C# fields are not extracted, so they are never reported. There is a test for it, so it will be noticed if field extraction is added.
  • Rules are documented in docs/PACKS.md.

• Proof: cargo test -p plumbgraph-core --test unity_entry_points passes 9/9 (Linux and Windows). Goldens unity_default.txt / unity_low.txt were blessed and reviewed by hand at each step; later steps changed only line numbers or added lines from new fixture files. Real code, v1.0.0 vs this branch, dead-code --include-low --max-findings 0, tier-0, no SCIP:

  repo (commit)	before	after
  Newtonsoft.Json 52fa3ae (non-Unity C#, control)	204 (11 high, 67 med, 126 low)	204, identical set (0 added, 0 removed)
  Unity open-project-1 608eac9, UOP1_Project/Assets/Scripts only	371 (5 high, 97 med, 269 low)	166 (3 high, 28 med, 135 low)

  On open-project-1, 203 symbols are no longer reported at any level. The top names are Start ×23, OnTriggerEnter ×7, OnGUI ×6, OnDestroy ×5 and OnValidate ×4; the rest became reachable through those. The 2 lost HIGH findings were checked by hand: LocalizeComponent_TMProFont (a [MenuItem] method) and ReplaceContextMenu ([InitializeOnLoadMethod] + [MenuItem]). For the remaining mediums I grepped 9 names: 7 (e.g. MenuController.OpenMenu/UnpauseMenu, private) occur only at their definition in any .cs file. These look like real leads. Index time on Newtonsoft (3 runs each): 1.13-1.21 s before, 1.14-1.28 s after, so no measurable change.

4. plumb map --budget fails while --tokens works

• Failing test: crates/cli/tests/cli.rs::map_budget_is_an_alias_of_tokens. Before: error: unexpected argument '--budget' found.
• Root cause: the flag was only declared as --tokens, although the help text, the output (budget 1500) and the docs all call it a budget.
• Fix: visible_alias = "budget". Help shows [alias: --budget]. Both flags at once is a usage error (exit 2), and the output is byte-identical with either flag. The README quickstart shows --tokens 1500 and mentions the alias. The MCP repo_map parameter stays tokens (unchanged).
• Proof: cargo test -p plumbgraph-cli --test cli map_. On real Windows, plumb map C:\...\Wraith --budget 200 gives # plumb map: 5 files, 18 of 264 symbols (~191 tokens, budget 200).

5. Windows install docs

• No test (docs). The README has a new ### Windows section: cargo install --path crates/cli --locked, and the PATH setup for %USERPROFILE%\.cargo\bin (rustup adds it; Chocolatey's Rust does not, which is exactly what happened here). It also states exactly what was tested. "No Windows testing" became "one machine, manual runs, no CI".
• Proof: on the Windows machine below, cargo install --path crates/cli --locked --root <tmp> built and installed plumb.exe/plumbgraph.exe for this branch, and v1.0.0 was installed the same way into ~/.cargo/bin. The PowerShell PATH snippet in the README was run as written; it is idempotent, and the user PATH still has exactly one .cargo\bin entry.

6. plumb init and .gitignore

init already appended .plumbgraph/, so the work here went into what was wrong with it.

• Failing tests: crates/core/tests/init_gitignore.rs. Before:
  • index_is_ignored_but_the_allow_list_stays_committable: allow.toml is documented as committed: .plumbgraph/, checked with real git check-ignore.
  • dry_run_reports_the_gitignore_lines_and_writes_nothing.
  • equivalent_existing_patterns_are_respected: a duplicate was appended after /.plumbgraph.
  • a_project_in_a_subdirectory_of_a_repo_gets_its_own_entry.
  • crlf_gitignore_keeps_its_line_endings.
  • outside_git_nothing_is_written_and_the_report_says_why.
  • a_gitignore_that_is_not_utf8_is_never_overwritten: user content must survive. read_to_string(..).unwrap_or_default() read a UTF-16 .gitignore (what PowerShell 5 echo > .gitignore writes) as empty and overwrote it.
  • Added in the review commit: !.plumbgraph/ and .plumbgraph* must be respected. Before, the appended .plumbgraph/* won as the last match and silently undid the user's negation.
• Root causes: dry-run only printed would write .gitignore; it wrote the directory form .plumbgraph/, which git cannot re-include files from; it used an exact-line equality check; it required <root>/.git; and it read the file with a lossy fallback.
• Fix:
  • Writes .plumbgraph/* + !.plumbgraph/allow.toml, with a comment line.
  • Any existing non-comment rule mentioning .plumbgraph means the user manages it, so the file is left alone.
  • Looks for .git in ancestors (as a dir, or as a file for worktrees and submodules).
  • Keeps CRLF and recognises a BOM.
  • Never rewrites a file that is not UTF-8; it reports what to add instead.
  • --dry-run prints note: would add to <path>/.gitignore: .plumbgraph/* and !.plumbgraph/allow.toml.
  • New --no-gitignore option (init_with).
  • The existing test init_writes_instructions_and_mcp_config_without_clobbering is unchanged and passes.
• Proof: cargo test -p plumbgraph-core --test init_gitignore passes 10/10 (Linux and Windows), and cargo test -p plumbgraph-cli --test cli init passes. Note: the CLI test init_dry_run_shows_the_gitignore_lines was written after the core fix, so it was not shown failing; the same behaviour was shown failing at core level.

Self-critique: what review found and fixed before opening this

• The Unity file gate first used source.contains("UnityEditor"). A comment in my own fixture triggered it and turned [MenuItem] in a non-Unity file into an entry point. It is now an AST check over using directives and qualified names; comments and strings are skipped. Covered by outside_assets_unity_context_comes_from_code_not_comments_or_strings and the Server/Worker.cs fixture.
• The Assets/ indirect rule accepted interface-only classes (Pool : IDisposable); now excluded (Pooled.Update is still reported). AddItemsToMenu was missing. ISerializationCallbackReceiver callbacks on a plain [Serializable] class (found on open-project-1, SceneSelector.Storage) are now covered by the callback-interface map. Each of these was a failing test first.
• .gitignore negations (above); a UTF-8 BOM.
• deadcode.rs module doc pointed at a non-existent docs/DEAD_CODE.md and omitted the existing subclass penalty; both fixed.

scripts/verify.sh (full, not --quick)

Run on a clean Linux checkout of this branch (WSL2 Ubuntu, toolchain 1.85.0 from rust-toolchain.toml, CI=true, cargo-deny and cargo-audit installed). Exit 0.

==> cargo fmt --check                        (no output)
==> cargo clippy (-D warnings)               Finished, no warnings
==> cargo test                               189 passed, 0 failed (per binary: 24 cli, 56 core unit, 7 deadcode_golden,
                                             6 deps_golden, 16 diagnostics, 10 init_gitignore, 6 init_snippet, 3 langs_go_java_cs,
                                             6 map_impact, 7 providers_exe_ext, 11 scip, 9 unity_entry_points,
                                             5 validation_regressions, 7 verify, 4 langs, 12 mcp)
==> test integrity                           test-integrity: OK (194 tests, baseline 194)
==> dogfood: plumb verify .                  dead-code 0, check-deps 0, test-weakening 0 -> verify: PASS (0 new)
==> cargo deny check                         advisories ok, bans ok, licenses ok, sources ok
                                             (5 warning[duplicate], 1 warning[license-not-encountered]; Cargo.lock,
                                             manifests and deny.toml are identical to main)
==> cargo audit                              Loaded 1296 advisories, scanned 168 crates, nothing reported
verify: OK

target/debug/plumb weakening --base origin/main --fail-on high: 0 finding(s) in 30 changed file(s) vs origin/main (98f17c191747), exit 0.
Test-count baseline raised 159 → 194 for the 35 added tests. No test was removed, skipped or loosened. The only edits to pre-existing test files are additions (cli.rs).

CI

All checks green on head ac8bf5a (run 38039417801, CodeQL run 38039417811):

check	result
verify (ubuntu-latest), full scripts/verify.sh with cargo-deny + cargo-audit, Rust 1.85.0	pass (1m16s). Log: test-integrity: OK (194 tests, baseline 194), dogfood verify: PASS (0 new ...), verify: OK
test-weakening gate (dogfood)	pass. 0 finding(s) in 30 changed file(s) vs origin/main (98f17c191747)
analyze (rust), analyze (actions), CodeQL	pass
GitGuardian Security Checks	pass

No workflow files were changed; the existing actions are already pinned to SHAs.

Not tested

• No Unity install. No Unity editor or player was run. The rules come from Unity's scripting reference, and were checked on source only: the fixture, plus the scripts folder of Unity's open-project-1. Scenes, prefabs, animation clips and .asset files were not checked out or parsed.

• Windows: real, but one machine: Windows 11 Home 64-bit (build 26200) with Rust 1.91.1 x86_64-pc-windows-gnu from Chocolatey. No rustup, so 1.85 was not used on Windows; the MSVC target was not tried; there is no Windows CI.
  On that machine cargo test --workspace --no-fail-fast passes every new test (the new test files were re-run natively on the final HEAD ac8bf5a: providers_exe_ext 7/7, init_snippet 6/6, init_gitignore 10/10, unity_entry_points 9/9, deadcode_golden 7/7, cli map_/init 2+2). 10 tests fail there, and the same 10 fail on unmodified main (checked in a separate worktree):

  • 3 in cli (closed_stdout_pipe_does_not_panic: Windows os error 232 instead of EPIPE; two LSP tests);
  • 6 in core diagnostics and 1 in mcp (lsp_through_mcp_when_allowed). The LSP fake server needs a real python3; on that machine python3 is the Microsoft Store stub.

  These are not addressed here. The Windows PATHEXT lookup on Linux CI is exercised through the injected extension list, plus one cfg(windows) test that only runs on Windows.

• verify --run on Windows: the diagnostics runner (builtin_tools) still starts tools by bare name. cargo/ruff work, because Command appends .exe, but npm .cmd shims such as tsc would not. Not changed here; it needs --allow-exec/--run testing on Windows.

• MSVC Windows, macOS, Unity packages under Packages/, the Editor/EditorWindow rules on a real editor-tools repo.

Risks

• Hidden real dead code in Unity projects. A component whose message methods are entry points is always live. A component that is never added to any scene can no longer be reported, because scenes are not indexed. The Assets/ indirect rule treats any class whose first base is a class as a possible component, so Update/Start/... on such a class are entry points even if it is not a MonoBehaviour. A base class named like an interface (IKController) would be misclassified the other way.
• Packages/ in a non-Unity .NET tree would turn on the attribute rules and the Editor/EditorWindow bases for C# files there.
• Private Unity methods called by Animation Events (which can target private methods) remain findings at medium. The fp_risk text is only on public methods.
• Schema/extractor bump: every existing .plumbgraph/index.db is rebuilt once on first use (existing mechanism, store::tests::version_mismatch_rebuilds).
• plumb init now writes .gitignore in a subdirectory of a repository (previously only at a repo root), and writes .plumbgraph/* + !.plumbgraph/allow.toml instead of .plumbgraph/. Repositories that already ran v1.0.0 init keep their .plumbgraph/ line; it is respected, not rewritten.
• PATHEXT entries other than .com/.exe/.bat/.cmd are ignored on purpose.