Skip to content

fix: v1.0.3 Unity partial-class messages, Unity notes, MCP/Windows hardening - #26

Merged
grloper merged 1 commit into
mainfrom
fix/v1.0.3-unity-partials
Oct 10, 2026
Merged

grloper merged 1 commit into
mainfrom
fix/v1.0.3-unity-partials

Conversation

@grloper

@grloper grloper commented Oct 10, 2026

Copy link
Copy Markdown
Owner

Fixes the false positives and install problems from a real run of plumb 1.0.2 on Assets/UYF (private repo). Not released, not tagged, version still 1.0.2; the CHANGELOG has an [Unreleased] 1.0.3 entry.

Per item

Every test below was written first and run against 1.0.2 behaviour (plus a stub where the new API had to exist to compile). Failure reasons are quoted from the output.

P0: Unity messages on partial classes

  • Failing test: unity_partials::partial_messages_are_entry_points_when_any_part_declares_a_unity_base. It failed with false positive Blob.LateUpdate, and the list also held Blob.OnDisable, AutoTest.FixedUpdate, Hero.LateUpdate.
  • Root cause: extraction is per file, and a part without a base list (partial class Blob { void LateUpdate() }) never looks like a Unity class.
  • Fix:
    • Class parts record their bases (new unity column on symbols).
    • Message methods of partial classes whose own part proves nothing are deferred to the dead-code pass.
    • That pass merges parts by type name per Assets/ root, follows the base chain through classes in the index (Hero : CharacterBase : MonoBehaviour), and makes all parts live.
    • dead-code --verbose prints debug: unity-message@partial-merged ... (also in JSON as notes; test partial_merge_leaves_a_debug_note and CLI test dead_code_verbose_prints_unity_partial_merge_notes).
  • Proof: cargo test -p plumbgraph-core --test unity_partials.

P0b: private non-message helpers stay reportable

  • Test: unreferenced_private_helpers_on_a_monobehaviour_are_still_reported (Net.LocalPortString, Net.NetExtraDeadHelper, Blob.BlobDeadHelper).
  • It was not a bug in the 1.0.2 code and passes with the merge in place; it guards that the merge does not over-suppress.
  • Note: Net.LocalPortString is medium 0.71 because of the existing 0.25 "method of a class with a base type" penalty. That is above the default 0.70 cutoff but not by much. I did not change that penalty.

P1: plain class Update(float dt)

  • Test: plain_class_update_is_not_an_engine_callback.
  • For a class with no base (TasterState), ordinary *Update* names and same-file MonoBehaviour.Update, 1.0.2 was already right. These are regression guards that passed before the change.
  • The failing case was a non-component class that derives from a plain base class under Assets/ (DerivedState : PlainBase). Failure: missing DerivedState.Update. 1.0.2 treated any non-interface base under Assets/ as possibly Unity.
  • Fix: the same deferred resolution walks the base chain; a chain that resolves to non-Unity classes is not a component.
  • main is not special-cased anywhere new.

Editor

  • Test: editor_bases_and_attributes. It failed with false positive Wizard.OnWizardCreate.
  • Fix: ScriptableWizard is an editor base, and OnWizardCreate/Update/OtherButton are messages.
  • [MenuItem], [InitializeOnLoad], [InitializeOnLoadMethod], [RuntimeInitializeOnLoadMethod], Editor and EditorWindow (including a partial window) were already handled; the test pins them. Startup.UnrelatedStaticHelper is still reported.

Severity

  • Test: unresolved_host_type_is_medium_with_a_reason_never_high. It failed because Mystery.Update was not reported at all (silently live).
  • Fix: when the host type's Unity base cannot be decided (a base type outside the index), the finding is capped at MEDIUM with the fp_risk unity-message-name-but-host-type-unresolved.
  • The test also asserts that HIGH on exact Unity message names occurs only on types proven non-Unity.

P1 docs / P3 footer

  • Docs: docs/PACKS.md now covers partial messages (removed from "Not covered" for messages), the editor rules, and a Unity gaps table: scenes/prefabs/UnityEvents, string SendMessage/Invoke (low confidence only), animation events, Addressables/Resources.Load, scip-dotnet opt-in, multi-asmdef.
  • Footer tests: unity_pack_footer_notes_when_indexing_under_assets and CLI dead_code_prints_unity_footer_under_assets. They failed first because the notes were absent. The notes appear only when C# files under Assets/ are indexed.
  • plumb dead-code --help links docs/PACKS.md#unity-c (CLI test dead_code_help_links_the_unity_pack_limitations, failed first).

P2 Windows / MCP

  • Startup line: mcp_startup_line_on_stderr_has_version_and_pid and the startup_line_has_version_and_pid unit test failed first (the old line had neither). It is now plumbgraph MCP server v<ver> pid=<pid> (stdio) root=... allow_exec=....
  • serverInfo.version: 1.0.2 already sent it. I added the missing assertion in handshake_and_list, which passed immediately.
  • plumb doctor warning:
    • The logic is providers::overwrite_warning(exe, probe), platform independent and tested in tests/self_overwrite.rs; two of the three tests failed against a stub.
    • The real probe is #[cfg(windows)] only.
  • README, docs/MCP.md: stop MCP hosts before cargo install on Windows (or use --root), and point plumb mcp --root at the current workspace repo.

Verification

  • scripts/verify.sh exits 0 (fmt, clippy -D warnings, 209 tests, test-integrity, dogfood plumb verify ., cargo-deny, cargo-audit).
  • Test-count baseline raised 194 to 209. No test was weakened, skipped or deleted.
  • plumb weakening --base origin/main: 0 findings.
  • Actions are unchanged and already SHA-pinned.
  • Real project (stretch), plumb dead-code on grloper/use-your-friends @43b8a8e:
symbol 1.0.2 this branch
Blob.LateUpdate HIGH not reported
Blob.OnDisable HIGH not reported
AutoTest.FixedUpdate HIGH not reported
Net.LocalPortString HIGH HIGH (kept)
TasterState.Update MEDIUM 0.77 MEDIUM 0.77 (kept)

Total findings 19 to 16.

Not tested / caveats

  • Behaviour change. A message on a class under Assets/ whose only base is outside the index (e.g. a DLL base class) used to be silently live. It is now reported as MEDIUM with the unresolved reason, as the spec asks. This can add noise on projects that use third-party base classes.
  • No Unity editor was run. The rules follow the scripting reference and fixtures, as in 1.0.1.
  • Merging.
    • Parts are keyed by simple type name within an Assets/ root. Namespaces and .asmdef assemblies are not told apart.
    • A same-named type in another namespace can over-suppress, which is the safe direction.
  • Windows.
    • The cfg(windows) doctor probe and its output were not compiled or run on Windows (no Windows target here); only the platform-independent logic is tested.
    • The cargo install file-lock behaviour is as reported by the user and documented, not reproduced here.
  • The MCP dead_code tool does not expose notes (the CLI and the core API do).
  • Index schema 4 / extractor 9: existing indexes rebuild once.
  • CI status: see the checks on this PR.

@grloper
grloper merged commit 0c5ebf3 into main Oct 10, 2026
6 checks passed
@grloper
grloper deleted the fix/v1.0.3-unity-partials branch October 10, 2026 10:04
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.

1 participant