fix(mcp): stop an unresolved reference from blocking safe_to_install

[goodhee]

Summary

• safe_to_install no longer fails when every exceptional ledger row is REFERENCE_UNRESOLVED
• require partially_inspected_files == 0, which is_complete used to cover
• regression tests for the caveat case and for the opaque-target case that must still block

A SKILL.md mentioning a file the bundle does not carry (typically a runtime
output, "save the result to out.md") resolves to nothing, so is_complete
goes false and a scan with zero findings and every file fully inspected is
reported unsafe to install. Unquoted save.md passes and the backticked form
does not, so the verdict tracks markup, not content. Other reason codes,
analyzer limitations, and partial or uninspected files still block.
is_complete, the report, and CAUTION are unchanged.

Fixes #510.

Testing

• uv run pytest tests/nodes/test_security_remediation.py tests/unit/test_mcp_server.py tests/nodes/test_finalize_inspection_ledger.py -q (393 passed)
• uv run ruff check src/ tests/
• uv run ruff format --check src/ tests/
• uv run mypy src/skillspector/mcp_server.py

[sylvesterkaczmarek]

REFERENCE_UNRESOLVED is emitted for both missing and ambiguous references in build_context.py, so reference_caveat_only also makes an ambiguous-reference scan safe_to_install=true. Those cases remain intentionally incomplete because the referenced target cannot be determined. Could we distinguish missing from ambiguous and exempt only the intended missing-reference case, with an ambiguous-reference regression test?

[rng1995]

[SkillSpector Review]

Reviewed head ad0e683c3dc6086bb7309f29b6302dd59ead6ff2 — REQUEST_CHANGES.

• src/skillspector/mcp_server.py:153-166: reference_caveat_only accepts every REFERENCE_UNRESOLVED ledger row, but that reason code represents both a missing target and an ambiguous target. An ambiguous reference can point at multiple bundled artifacts, so the scanner has not established which bytes the instruction reaches; nevertheless this branch can return safe_to_install=true. Preserve the resolution status in the completeness data or introduce distinct missing/ambiguous reason codes, exempt only the intended missing-output case, and add an MCP regression test proving an ambiguous reference remains unsafe.

Required checks pass, but the requested correction and mergeStateStatus=BEHIND both block merging.

[ikaros-0117]

As the reporter of #510, I agree with the missing-only scope proposed in the review. Ambiguous references should remain blocking, and I'm fine with narrowing the original issue wording accordingly.

[goodhee]

Good catch, both of you — REFERENCE_UNRESOLVED was indeed overloaded for two different situations. Fixed by splitting it:

• Added a new LedgerReason.REFERENCE_MISSING for the "target doesn't exist in the bundle" case (e.g. a file written at runtime).
• LedgerReason.REFERENCE_UNRESOLVED now means only "ambiguous" — matched more than one bundled artifact, target undetermined.
• reference_caveat_only in mcp_server.py now checks for REFERENCE_MISSING specifically, so an ambiguous reference keeps blocking safe_to_install like any other exceptional reason.

Added test_ambiguous_reference_caveat_still_blocks_mcp_install: two files sharing a basename (docs/guide.md, extra/guide.md) referenced by the bare name, asserting safe_to_install is False and reason_code == "reference_unresolved". Updated the existing missing-reference test to expect the new reference_missing code.

Branch has been rebased onto latest main.

[sylvesterkaczmarek]

Rechecked the current head. My earlier concern is resolved: missing references now use a distinct REFERENCE_MISSING reason that alone can receive the MCP caveat, while ambiguous references remain REFERENCE_UNRESOLVED and continue to block safe_to_install. The added ambiguous-reference regression covers the unsafe case directly. No remaining blocker from my review.

[rng1995]

[SkillSpector Review]

Re-reviewed current head f854738d12abe1832b59b28e662832bc1b0963b1. The prior install-gating issue is resolved: missing references now use REFERENCE_MISSING, while ambiguous/opaque targets remain REFERENCE_UNRESOLVED; safe_to_install only permits the missing-reference exception when there are no limitations, partially inspected files, entirely uninspected files, or execution failure. The added regressions cover the safe missing-reference case and both unsafe unresolved cases. I found no remaining required code, test, documentation, security, or compatibility changes.

Required checks are green. Merge remains blocked by the PR's current conflicts (mergeable=CONFLICTING, mergeStateStatus=DIRTY).

[sylvesterkaczmarek]

Rechecked current e85ebc5a after the merge from main. The PR-specific diff remains the same four files, the missing-vs-ambiguous reference fix is intact, and required checks are green. No new blocker from me.

[rng1995]

[SkillSpector Review]

Re-reviewed exact head e85ebc5aa6764e438a919400fa3f8ba6cab74326 after the main synchronization. The prior install-gating issue remains resolved: missing references use REFERENCE_MISSING, ambiguous targets remain REFERENCE_UNRESOLVED, and the MCP exception applies only when every exceptional row is missing-reference-only and there are no limitations, partially inspected files, entirely uninspected files, or execution failure. The focused safe-missing, unsafe-ambiguous, and unsafe-opaque regressions remain present.

All exact-head hosted checks pass, GitHub reports the branch mergeable and clean, and I found no remaining required code, test, documentation, security, or compatibility changes on this head.