Skip to content

fix(skills_sync): honor .no-bundled-skills marker for DESCRIPTION.md copy (finish PR #15) - #22

Merged
sahilm-ti merged 1 commit into
mainfrom
kanban/t_ad76c02b
May 26, 2026
Merged

fix(skills_sync): honor .no-bundled-skills marker for DESCRIPTION.md copy (finish PR #15)#22
sahilm-ti merged 1 commit into
mainfrom
kanban/t_ad76c02b

Conversation

@sahilm-ti

Copy link
Copy Markdown
Owner

What

Defense-in-depth check inside sync_skills() for the .no-bundled-skills opt-out marker. When present at HERMES_HOME (resolved as SKILLS_DIR.parent), sync_skills() short-circuits with skipped_opt_out=True before either the SKILL.md or DESCRIPTION.md copy loops run.

Why

PR #15 added marker checks at the documented callers (gateway startup, the update command, seed_profile_skills) but the DESCRIPTION.md shutil.copy2 loop at tools/skills_sync.py:325 ran unconditionally inside sync_skills() itself. As a result, opted-out profiles (e.g. braintrusteng, bt-optimizer) still accumulated byte-identical DESCRIPTION.md stubs under <profile>/skills/<category>/ that duplicate ~/.hermes/skills/<category>/DESCRIPTION.md. agent/prompt_builder.py:1162 reads category descriptions from the external (global) dir already, so the profile stubs were dead bytes.

braintrustOrch profile (no stubs) has worked correctly throughout — proof the profile-side DESCRIPTION.md files are unnecessary.

Cleanup

Removed existing stubs from both braintrusteng and bt-optimizer skills dirs (14 files total, all verified byte-identical to global before deletion). braintrusteng's now-empty top-level category dirs were also removed; bt-optimizer keeps its category dirs (they hold real custom skills).

Tests

  • New TestNoBundledSkillsMarker class with two cases: marker-present skips both copies, marker-absent proceeds normally.
  • Fixed test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean tmp path so the developer's live opt-out marker doesn't short-circuit the test.
  • pytest tests/tools/test_skills_sync.py -q → 47 passed.

Refs: PR #15 (t_00313e4b), kanban t_ad76c02b

…copy (finish PR #15)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@sahilm-ti, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 58 minutes and 1 second. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ae8553c4-74e1-4d00-82e5-ea17fae69506

📥 Commits

Reviewing files that changed from the base of the PR and between d78791a and be297e3.

📒 Files selected for processing (2)
  • tests/tools/test_skills_sync.py
  • tools/skills_sync.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kanban/t_ad76c02b

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

🔎 Lint report: kanban/t_ad76c02b vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9377 on HEAD, 9377 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4956 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@sahilm-ti

Copy link
Copy Markdown
Owner Author

auto-review: approved, awaiting human merge + kanban_approve.

Rules checked:

  • U1 in-scope: tools/skills_sync.py + tests/tools/test_skills_sync.py — matches AC
  • U2 deletions: additions-only diff
  • U3 secrets: none
  • U4 AC coverage: installer path patched (sync_skills marker guard), regression test added (TestNoBundledSkillsMarker), profile stub cleanup verified on disk (no DESCRIPTION.md under braintrusteng/bt-optimizer skill dirs)
  • U5 mergeable: MERGEABLE / UNSTABLE
  • C1 CI: all required checks green (lint, ruff diff, ruff enforcement, Windows footguns, 6 test shards, e2e, nix x2, supply chain, attribution)
  • C2 type-discipline: no new type: ignore / cast
  • C4 tests touched: yes

@sahilm-ti
sahilm-ti merged commit f0b371a into main May 26, 2026
22 checks passed
@sahilm-ti
sahilm-ti deleted the kanban/t_ad76c02b branch May 26, 2026 10:12
sahilm-ti added a commit that referenced this pull request May 28, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request May 28, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request May 28, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request May 29, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jun 3, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jun 5, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jun 15, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jun 17, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jun 22, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 3, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 9, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 10, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 11, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 13, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 15, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 17, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 21, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 23, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Jul 28, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Aug 24, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
sahilm-ti added a commit that referenced this pull request Sep 2, 2026
…copy (finish PR #15) (#22)

PR #15 added the .no-bundled-skills marker check at every documented
caller of sync_skills(), but the DESCRIPTION.md copy loop inside
sync_skills() ran unconditionally — so when a profile-bootstrap path
that *had* the marker check still ended up calling sync_skills()
directly (e.g. seed_profile_skills() racing the marker write, or a
caller path PR #15 didn't cover), the SKILL.md copy was correctly
skipped but the category DESCRIPTION.md files still landed in the
profile skills dir.

Result: opted-out profiles (braintrusteng, bt-optimizer) accumulated
useless byte-identical DESCRIPTION.md stubs under */skills/<category>/
that duplicate the global versions at ~/.hermes/skills/<category>/.
agent/prompt_builder.py reads category descriptions from the external
(global) dir already (line 1162), so the profile stubs are dead bytes.

Fix is defense-in-depth: sync_skills() itself now checks the marker at
HERMES_HOME (SKILLS_DIR.parent) and short-circuits to a zero-effect
return with skipped_opt_out=True before either loop runs. Regression
tests cover both paths (marker present skips everything; marker absent
proceeds normally).

Also fixes test_nonexistent_bundled_dir to patch SKILLS_DIR to a clean
tmp path so the developer's live opt-out marker doesn't short-circuit
the test.

Refs PR #15 (t_00313e4b), kanban t_ad76c02b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant