fix(security): enforce discovery and analysis completeness

[Spectorian]

Summary

• include bounded dot-prefixed child skills while preserving explicit skip directories and link safeguards
• keep requested and executed optional-analysis state distinct across reports and MCP output
• require a complete requested analysis pass for an install-safe MCP verdict while preserving explicit static-only behavior

Validation

• 276 focused and adversarial regressions
• 2,806 non-integration/non-provider tests
• cross-surface JSON and MCP accounting checks
• Ruff lint, format check, targeted mypy, and git diff --check

[rng1995]

Requesting changes because the requested semantic pass can still be skipped entirely while this PR reports it as used and complete, producing an install-safe verdict. The current-head end-to-end repro executes zero semantic calls but returns llm_used=True, scan_mode=static+llm, recommendation=SAFE, and safe_to_install=True.

[mohgupta-ship-it]

Powered by Codex: correction after a five-lens GPT-5.6-sol council on current head 1b252b5.

• Rating: critical fix
• Disposition: NEEDS_CHANGES
• Critical gap: yes — hidden-content privacy and incomplete-analysis gate integrity are violated.
• CI/live state: all five hosted checks are green; mergeable is true, but review state is blocked.
• Council method: independent spec/regression, security/trust-boundary, runtime/architecture, standards/YAGNI, and focused-test lenses; an evidence-only judge then adjudicated reproduced claims.

This supersedes the prior mergeable assessment. New, non-duplicate findings:

1. Dot-child discovery + recursive re-rooting erases hidden ancestry and moves content from local-only storage into the provider-facing LLM cache.
2. Fully successful root+child semantic telemetry is falsely degraded because status cardinality is enforced globally instead of per source scope.
3. Missing semantic telemetry leaves canonical JSON analysis_completeness complete, allowing --fail-on-incomplete to exit successfully despite degraded LLM coverage.

flowchart LR
  A[".hidden-skill/SKILL.md"] --> B["dot-child discovery"]
  B --> C["recursive re-root"]
  C --> D["path becomes SKILL.md"]
  D --> E["llm_file_cache"]
  E --> F["external semantic provider"]

Loading

New anchored review: #410 (review)

Recommended action: do not merge until hidden/local-only provenance survives recursive re-rooting and semantic incompleteness is source-aware and canonical.

[Spectorian]

Fixes pushed. please review and confirm.

[mohgupta-ship-it]

Powered by Codex: five-lens GPT-5.6-sol delta council — REQUEST CHANGES on 1b252b5. The prior mergeable assessment is superseded. The new dot-child path crosses the documented hidden-content/LLM trust boundary, and semantic completeness is inconsistent across transitive reporting and the CLI gate.

[rng1995]

[SkillSpector Review]

Re-review: the original empty-telemetry blocker is addressed and its focused regressions pass, but the current head still crosses the hidden-content trust boundary when a dot-prefixed child is re-rooted. It also treats two independently complete root/child semantic scopes as incomplete—I reproduced (True, False)—and does not project semantic failure into canonical analysis_completeness. Those current blockers are already anchored in live threads, and the branch conflicts with main.

[rng1995]

Implemented and validated the complete review pass in 32695c2, after rebasing onto latest main (03e8f56) without merge commits.

Architectural fixes:

• Made dot-child local_only provenance immutable/monotonic through recursive and transitive scans.
• Enforced privacy at the provider-cache boundary while retaining deterministic local analysis; partitioned transitive caching by target and privacy mode.
• Source-bound semantic calls and statuses, with per-(source_identity, analyzer_id) accounting and same-scope duplicate rejection.
• Projected missing semantic runtime evidence into the canonical ledger/completeness model, shared by renderers, CLI exit policy, and MCP install decisions.
• Preserved the credential-independent graph design while restoring the latest-main compatibility export.

Validation:

• 4878 passed, 14 skipped, 38 deselected, 4 xfailed for the full non-provider/non-integration suite.
• 28 passed for non-live integration coverage.
• Ruff lint and format checks passed.
• Source distribution and wheel built successfully.
• GitHub CI: changes, DCO, lint, unit tests, and Docker smoke all pass.

All four review threads received specific fix/test replies and were resolved.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 32695c2382b8d7158d1c543277f366583b85109e after its rebase onto current main. The prior blockers are resolved: semantic analyzers remain credential-independent at graph construction; empty or malformed runtime telemetry fails closed; dot-child local-only provenance is preserved through recursive and transitive scans without entering provider or meta-analysis payloads; root and child telemetry is source-bound and validated independently; and unmet requested semantic work is projected into the canonical completeness model consumed by JSON, SARIF, CLI, and MCP install decisions.

The focused regressions cover privacy-mode cache isolation, hidden-child provider exclusion, root/child telemetry, missing telemetry, CLI exit behavior, and MCP safety decisions. All hosted checks pass and no unresolved review threads remain.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 1c685c4e1a3c73d09013afde1bd12ea396d3ddf9 after the automated main synchronization. This is a clean merge of approved head 32695c2382b8d7158d1c543277f366583b85109e with f161b700d83d1e685f501c525d4cf7a8a49bd82e (#451); the only new content is #451's reference-matcher change and its regressions, with no conflict-resolution edits. The PR-owned completeness/privacy delta is unchanged, so the prior resolution assessment remains valid: local-only provenance and provider isolation, source-bound semantic telemetry, fail-closed runtime accounting, and canonical CLI/MCP/report completeness gates remain intact. No unresolved review threads remain.

Approval applies to this exact head. It is not merge-ready yet: CI run 34934104937 concluded action_required without producing any checks, and another reviewer's active Changes Requested state remains.

Dismissed after exact-head re-review at c71d341. The hidden/local-only provider boundary, root/child source-bound semantic telemetry, and canonical fail-closed completeness contract are all preserved. The main-sync conflict was resolved by composing local-only exclusion with bounded truncated public-source handling, with an interaction regression. All 4 threads are resolved and the full exact-head CI suite is green.

[rng1995]

[SkillSpector Review]

Approved exact head c71d3414cb41120a92e9ead2af4a02744cb055a1 after independent re-review of the security-discovery completeness changes and the main-sync conflict resolution. All 4 review threads are resolved. Local-only/hidden content remains excluded from provider payloads; root and child semantic telemetry is source-bound and fails closed; incomplete semantic execution flows into the canonical report, CLI, and MCP safety gates. The conflict resolution correctly composes that privacy boundary with bounded handling of truncated public source, and its interaction regression passes.

The focused 457-test PR suite plus merge-interaction tests, Ruff, format, and diff checks passed, and the complete exact-head hosted CI suite is green. No blocking defect remains.