Skip to content

fix(skills): persist quarantined trust row for AutoSkill drafts at generation time - #6705

Merged
bug-ops merged 1 commit into
mainfrom
fix/6702-skill-quarantine-shadow
Jul 28, 2026
Merged

fix(skills): persist quarantined trust row for AutoSkill drafts at generation time#6705
bug-ops merged 1 commit into
mainfrom
fix/6702-skill-quarantine-shadow

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

SkillGenerator::write_quarantined wrote an AutoSkill trace-extraction draft's SKILL.md to _quarantine/<name>/ but never persisted a skill_trust DB row. The doc comment claimed the _quarantine directory prefix kept the registry from ever loading these drafts, but the loader applies no directory-name filtering at all — quarantine was never actually enforced at the trust layer. Because no DB row existed yet, the draft's first hot-reload had no existing row to preserve and fell through to [skills.trust] default_level, silently trusting an unreviewed draft whenever an operator set default_level = "trusted". Separately, AutoSkill-generated names had no collision check against native tool IDs.

  • trace_extraction::log_and_persist now eagerly writes a Quarantined skill_trust row right after generation, keyed on the name actually written to disk (both Add and Merge branches), before the skill can ever reach a hot-reload.
  • A guard skips the write (and warns) instead of overwriting when the target name already has a trust row pointing at a different on-disk path — the merge LLM is prompted to preserve the existing skill's name, so a merge landing on an already-Bundled or -Trusted skill is the dominant case, not an edge case, and must not clobber that skill's trust row.
  • The native-tool-ID name-collision warning is extracted into a shared helper called from both the startup catalog emit and every hot-reload, so it no longer depends on SemanticMemory being configured and fires at startup rather than only after the first post-startup file change.
  • Corrected the false _quarantine "security boundary" doc comment on write_quarantined.

Closes #6702

Test plan

  • cargo nextest -p zeph-skills: 590/590 passed
  • cargo nextest -p zeph-core: 2232/2232 passed (2 skipped)
  • cargo nextest --workspace --lib --bins (CI feature set): 15269/15269 passed
  • cargo +nightly fmt --check, cargo clippy --profile ci --workspace --all-targets --features "desktop,ide,server,chat,pdf,scheduler,testing" -- -D warnings, rustdoc gate, gitleaks protect --staged all clean
  • New regression-lock tests: eager quarantine survives a hot-reload despite default_level = trusted; merge into an already-Trusted skill resets to Quarantined; merge/add against a differently-pathed Bundled/Trusted skill leaves its trust row untouched and warns instead; native-tool-ID collision warning fires with and without SemanticMemory configured, from both the startup and hot-reload call sites
  • .local/testing/playbooks/autoskill-a1-a2.md updated with 3 new live-session scenarios (not yet run live this cycle) and a corrected "Known Edge Cases" note; .local/testing/coverage-status.md row updated in place

…neration time

SkillGenerator::write_quarantined wrote an AutoSkill draft's SKILL.md but
never persisted a skill_trust DB row, so its first hot-reload had no
existing row to preserve and fell through to skills.trust.default_level,
silently trusting an unreviewed draft whenever an operator set
default_level = "trusted". The _quarantine/ directory naming had no
effect on the loader and never actually enforced quarantine.

log_and_persist now eagerly writes a Quarantined skill_trust row right
after generation, keyed on the name actually written to disk, with a
guard that skips (and warns on) writing over an unrelated skill's trust
row when a merge targets an existing Bundled or Trusted skill under a
different path. The native-tool-ID collision warning is extracted into
a shared helper so it fires at startup as well as on hot-reload, and no
longer depends on SemanticMemory being configured.

Closes #6702
@github-actions github-actions Bot added documentation Improvements or additions to documentation skills zeph-skills crate rust Rust code changes core zeph-core crate bug Something isn't working size/XL Extra large PR (500+ lines) labels Jul 28, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 28, 2026 18:04
@bug-ops
bug-ops merged commit cb54c15 into main Jul 28, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/6702-skill-quarantine-shadow branch July 28, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core zeph-core crate documentation Improvements or additions to documentation rust Rust code changes size/XL Extra large PR (500+ lines) skills zeph-skills crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AutoSkill trace-extraction drafts shadow native tool names and _quarantine/ has no effect on loading

1 participant