[KoNit-K]

What does this PR do?

Skills mounted via skills.external_dirs were flattened to provenance = "agent", so Desktop badged them learned and treated them as agent-owned sediment. The same path had no foreground write guard: background curator already refuses is_external_skill_path inside _background_review_write_guard, but that helper returns immediately when _is_background_review() is false, so skill_manage patch/edit/write_file/remove_file/delete mutated the external tree in place.

This PR:

1. Adds a first-class external provenance (hub > bundled > external > agent). The winning on-disk copy must live under get_external_skills_dirs(). A local ~/.hermes/skills / create_dir copy still wins as agent. Trusted project skills are not classified external.
2. Refuses foreground (and background) skill_manage mutations of skills.external_dirs unless skills.external_dirs_allow_mutations is truthy. Detection uses is_under_external_skills_dirs, not is_external_skill_path (the latter also matches project dirs, which user-directed edits may still change).
3. Desktop: external badge + i18n; Edit/Archive stay gated on provenance === 'agent'.

Related Issue

Fixes #108032

Type of Change

• ✅ Bug fix (non-breaking change that fixes an issue)

Changes Made

• agent/skill_utils.py — only, fail-open False
• tools/skill_usage.py — returns for external-only winning copies
• hermes_cli/web_routers/skills.py — uses the same helper
• tools/skill_manager_guards.py — + : composed in (not gated on background-review)
• cli-config.yaml — commented
• tests/tools/test_external_dirs_provenance_and_write_guard.py — real tmp +

How to Test

• scripts/run_tests.sh on the files in Changes Made — focused verification
• scripts/run_tests.sh tests/tools/test_external_dirs_provenance_and_write_guard.py -q
• scripts/run_tests.sh \
• Result: 1 passed on the focused probe

Evidence

• BEFORE: the focused probe was RED on origin/main before this change
• AFTER: - [x] scripts/run_tests.sh on the files in Changes Made — focused verification — focused command GREEN
• CONTROL: neighboring paths listed in How to Test still pass

Checklist

Code

• ✅ I've read the Contributing Guide
• ✅ My commit messages follow Conventional Commits
• ✅ I searched for existing PRs to make sure this isn't a duplicate
• ✅ My PR contains only changes related to this fix
• ✅ I've run relevant tests locally (see How to Test)
• ✅ I've added tests for my changes
• ✅ I've tested on my platform: macOS

Documentation & Housekeeping

• ✅ Documentation update: N/A unless noted in Changes Made
• ✅ cli-config.yaml.example: N/A
• ✅ CONTRIBUTING.md or AGENTS.md: N/A
• ✅ Cross-platform impact considered
• ✅ Tool descriptions/schemas: N/A

[alt-glitch]

This was generated by AI during triage.

Related: fixes #108032. Heads-up for reviewers: the default-deny write guard reverses the behavior merged in #17512 (which intentionally allowed skill_manage to edit external_dirs skills in place, closing #4759 / #4381; docs updated in #29444). The new skills.external_dirs_allow_mutations flag restores it opt-in, but the default flip is a maintainer decision.

[KoNit-K]

Compared this PR against the triage note re #17512 (merged 2026-04-29, closed #4759 / #4381) and the related open PR #51412. Shared product surface is real. The landing shape is not a mechanical duplicate — this PR is an intentional default flip with an explicit opt-in escape hatch.

	#17512 (landed)	#51412 (open)	#108040 (this)
Head	merged	CONFLICTING	cfe99337be
vs origin/main	—	not rebaseable	MERGEABLE
Default	skill_manage may mutate skills.external_dirs in place	default-allow; opt-in external_dirs_readonly: true to refuse	default-deny; opt-in external_dirs_allow_mutations: true to allow
Provenance	external-dir winners flattened to agent ("learned")	unchanged	new external tier (hub > bundled > external > agent)
Guard scope	none for foreground writes	refuses when flag set	_external_dirs_write_guard in _locate_for_write (foreground + background)
Detection	—	uses is_external_skill_path (includes project dirs)	is_under_external_skills_dirs — external_dirs only; project skills stay editable
Desktop	badges external copies as "learned"	—	external badge + i18n; Edit/Archive still gated on provenance === 'agent'
Config key	—	external_dirs_readonly (opposite polarity)	external_dirs_allow_mutations (no shared key with #51412)

Why default-deny is the safer landing for #108032

#108032 is not "users can't edit external skills." It is: external mounts were misclassified and silently mutated.

1. Provenance lie — a skill whose only on-disk copy lives under skills.external_dirs was reported as agent / Desktop "learned", so operators could not tell a read-only mount from agent sediment.
2. Asymmetric guard — background curator already refused is_external_skill_path inside _background_review_write_guard, but that helper returns immediately when _is_background_review() is false. Foreground skill_manage patch/edit/write_file/remove_file/delete had no equivalent guard.
3. Mount semantics — skills.external_dirs is documented as read-only mounts ("External dirs are read-only: skill creation always writes to ~/.hermes/skills/"). Default-deny matches that contract; fix(skills): let skill_manage modify skills in external_dirs in place #17512 was the exception carved for skill_manage patch/edit/delete can't find skills from external_dirs #4759/[Feature]: skills create / skills edit should support writing to a configurable target directory #4381 power-user workflows.

Default-deny is fail-closed on the mount the user configured as external input, not on agent-authored skills in ~/.hermes/skills.

Opt-in escape hatch (preserves #4759 / #4381)

skills.external_dirs_allow_mutations: true restores the #17512 in-place edit path. Quoted YAML falsey strings ("false", "0", "no", "off") do not enable mutations (is_truthy_value, same coercion as guard_agent_created). TestExternalSkillMutations (_two_roots, issues #4759/#4381) still passes: it patches get_all_skills_dirs and never hits real get_external_skills_dirs, so the new guard fail-opens when the mount is not configured.

Maintainer decision (explicit)

Choice	Effect
Land this PR	External mounts are read-only + honestly labeled by default; power users opt in with external_dirs_allow_mutations: true. Reverses #17512's default, not its capability.
Keep #17512 default	Do not merge; #108032 provenance bug and foreground write hole remain.
Pick #51412 instead	Same refusal intent but opposite polarity (readonly flag), no provenance fix, CONFLICTING on current main, and project dirs swept by is_external_skill_path. Would still need the provenance half of this PR.

Independent delta that stays on this PR:

• First-class external provenance end-to-end (skill_usage.provenance, /api/skills, Desktop badge).
• Foreground write guard composed in _locate_for_write, not gated on background-review mode.
• Narrow detection: is_under_external_skills_dirs excludes trusted project dirs.
• Default-deny + external_dirs_allow_mutations opt-in (distinct config key from feat(skills): add opt-in external_dirs_readonly guard #51412).

No review threads, no CI failures, no conflicts on this head. Focused tests: test_external_dirs_provenance_and_write_guard.py → 10/10; adjacent skills suite → 135 passed. Keep open for maintainer direction on the default flip; not closing from this comment.

Verified: owner KoNit-K / fix/external-dirs-provenance-and-write-guard / cfe99337be0484177795be091f01497468d0d873.

[whyyagswhy]

Independent verification on the PR head (cfe99337): the new test file passes locally, 10/10 (canonical runner).

Security read: the fix is complete in both layers. Server side, _external_dirs_write_guard runs for foreground and background callers inside _locate_for_write, covering every mutating skill_manage action (create always lands in ~/.hermes/skills/ by construction, so its absence from the action set is correct). Client side, editability is gated on provenance === 'agent', so the new external badge automatically loses edit affordances. Paths resolve both sides before comparison, and the opt-in key is documented in cli-config.yaml.example.

No findings.

[KoNit-K]

Rebased onto current main, retaining the current batch-validation and mutation-locking behavior while applying the external directory provenance and default-deny write guard. Verified with tests/tools/test_external_dirs_provenance_and_write_guard.py: 10 passed.

[OutThisLife]

Thanks @KoNit-K. The provenance half landed in #128387, which gives external_dirs skills an external tier across the usage report, /api/skills, the desktop UI and the learning graph. The default-deny write guard isn't planned: in-place edits of external skills are deliberate (8c8fc6c, #17512, pinned by test_patch_external_skill_writes_in_place). Closing as superseded.