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.

[rng1995]

[P1] Do not count an empty call log as a completed LLM pass. If the graph is imported before provider credentials are available, credential-gated semantic nodes are omitted. Binding a supported provider later through use_provider makes preflight succeed, but no semantic nodes or calls run and llm_call_log remains empty. This branch then returns (used=True, complete=True), allowing a SAFE/safe_to_install=True result even though meta_analysis_applied=False. Treat an empty log as not used/incomplete, or retain the expected analyzers and account their explicit statuses before allowing an install approval.

[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.

[mohgupta-ship-it]

Powered by Codex: [P0] Including a dot-prefixed child and then recursively re-rooting it erases the hidden path segment used by build_context to enforce local-only handling. A harmless marker was excluded from llm_file_cache as .review-helper/SKILL.md in the parent, but appeared in llm_file_cache as SKILL.md after child re-rooting. This contradicts the documented invariant that hidden content is never sent to an external LLM. Carry immutable root-relative hidden/local-only provenance into the child scan and add a mocked-provider test proving the marker never reaches an LLM batch.

[mohgupta-ship-it]

Powered by Codex: [P1] This global cardinality check rejects valid transitive telemetry. One successful root scope returns (used=True, complete=True); adding one equally successful child scope returns (True, False) because both retain the same analyzer IDs. Validate one terminal status per (source_identity, analyzer_id) (and source-bind call records), or validate each child before flattening. Keep true duplicate-within-scope rejection.

[mohgupta-ship-it]

Powered by Codex: [P1] Semantic-runtime failure is only reflected in degraded metadata/recommendation, not canonical analysis_completeness. With requested analysis and missing telemetry, the report is CAUTION and llm_degraded=True but JSON still says is_complete=True/status=complete; consequently --fail-on-incomplete can exit successfully. Project this failure into the canonical ledger/completeness result before rendering and add CLI exit-code coverage.