Skip to content

fix(skill-creator): do not hardcode author in template and docs#371

Merged
Alan-TheGentleman merged 1 commit into
Gentleman-Programming:mainfrom
Basparin:fix/98-skill-creator-author-placeholder
Jun 10, 2026
Merged

fix(skill-creator): do not hardcode author in template and docs#371
Alan-TheGentleman merged 1 commit into
Gentleman-Programming:mainfrom
Basparin:fix/98-skill-creator-author-placeholder

Conversation

@Basparin

@Basparin Basparin commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

🔗 Linked Issue

Closes #98

🏷️ PR Type

  • type:bug — Bug fix (non-breaking change that fixes an issue)

📝 Summary

The skill-creator SKILL.md template and the Frontmatter Fields documentation table both hardcoded metadata.author: gentleman-programming. As a result, every skill generated by the AI through this skill inherited the wrong author. The reporter mentioned 17 skills mis-attributed before noticing.

Following your approval comment (use a placeholder or read from git config user.name):

  • Template (line ~49): author: gentleman-programmingauthor: "{your-github-username}"
  • Docs table (line ~111): value row replaced with an instruction for the AI to infer the author from git config user.name or ask the user — never hardcode
  • Associated golden snapshots for Claude, OpenCode, Windsurf, and Kiro are updated to reflect the asset change

🏛️ Architectural decision (pinged you in the issue)

The issue lists 3 locations. This PR touches 2 (template + docs table) and intentionally keeps the skill-creator's own frontmatter (line ~8) as author: gentleman-programming, because the skill-creator IS your official skill — changing it to a placeholder would be false. The bug is that GENERATED skills inherit the wrong author, not the upstream skill itself.

Asked you in the issue comment to confirm this reading. Happy to extend to the 3rd location if you prefer.

📂 Changes

File / Area What Changed
internal/assets/skills/skill-creator/SKILL.md Template placeholder + docs row replaced
testdata/golden/skills-claude-skill-creator.golden Mirror of asset change
testdata/golden/skills-opencode-skill-creator.golden Mirror of asset change
testdata/golden/skills-windsurf-skill-creator.golden Mirror of asset change
testdata/golden/skills-kiro-skill-creator.golden Mirror of asset change

Total: 5 files, +10/-10 lines.

🧪 Test Plan

Unit Tests (ran locally)

go test ./internal/components/ -run TestGoldenSkills
  • TestGoldenSkills_Claude — PASS
  • TestGoldenSkills_OpenCode — PASS
  • TestGoldenSkills_Windsurf — PASS
  • TestGoldenSkills_Kiro — PASS
  • go vet ./... clean on touched packages

E2E Tests (Docker required)

Not run

Pre-existing Windows unit test failures (NOT introduced by this PR): TestSkillPathForAgent (path separator), TestGoldenEngram_Antigravity (snapshot drift on main), and several TestInjectOpenCode* tests requiring bun add unique-names-generator in test temp dirs. Verified these fail identically on a clean origin/main checkout.

✅ Contributor Checklist

⏳ Pending maintainer actions

  • Apply type:bug label to this PR (contributor cannot self-apply; CI validation blocks merge until then)
  • Confirm the 2-vs-3-locations architectural decision in the issue comment

Summary by CodeRabbit

  • Documentation
    • Updated the skill creator template to use a customizable placeholder for the author metadata field, making it clearer which values require personalization.

@Alan-TheGentleman Alan-TheGentleman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a focused bugfix, but the process gates need to be current.

Please add the required type:bug label, confirm the approved issue link, and refresh CI against current main.

@Basparin

Copy link
Copy Markdown
Contributor Author

@Alan-TheGentleman refreshed against current main → merge commit 6f646f7.

What changed during the rebase

Main rewrote SKILL.md to the new LLM-first style (shorter, with Activation Contract / Hard Rules / Decision Gates / Execution Steps sections). The rewrite partially fixed #98: the docs frontmatter table now reads "infer from git config user.name or ask the user. Do NOT hardcode." — so that part of this PR is superseded by main.

But the rewrite reintroduced the bug in the new inline template (around line 61):

metadata:
  author: gentleman-programming   # <- still hardcoded in main

This PR's fix re-applies cleanly to the new template line. Net diff vs upstream/main:

internal/assets/skills/skill-creator/SKILL.md        | 2 +-
testdata/golden/skills-claude-skill-creator.golden   | 2 +-
testdata/golden/skills-kiro-skill-creator.golden     | 2 +-
testdata/golden/skills-opencode-skill-creator.golden | 2 +-
testdata/golden/skills-windsurf-skill-creator.golden | 2 +-

One line per file — exactly the placeholder swap.

Local validation

go test ./internal/components/ -run TestGoldenSkills -v
--- PASS: TestGoldenSkills_Claude
--- PASS: TestGoldenSkills_OpenCode
--- PASS: TestGoldenSkills_Windsurf
--- PASS: TestGoldenSkills_Kiro
ok  github.com/gentleman-programming/gentle-ai/internal/components 0.676s

Goldens regenerated with -update after the merge; the diff matches exactly the template line being fixed.

Pending maintainer action

  • Apply the type:bug label (the only blocker I can't unblock as an external contributor).

Architectural question from the issue #98 comment is now moot — main already handled the docs table reading. This PR is now strictly scoped to the inline template fix. Happy to extend if you want the skill-creator's own frontmatter (author: gentleman-programming at line 6) also placeholdered, but I left it intact on the original architectural reasoning that the upstream skill IS yours.

@Alan-TheGentleman Alan-TheGentleman added the type:bug Bug fix label May 22, 2026
@Alan-TheGentleman

Copy link
Copy Markdown
Contributor

@Basparin — heads-up: this PR's Unit Tests failure is TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets. That test was rewritten in main by PR #660 (fix(agents/pi): use positional pnpm dlx argument, landed in v1.31.0) to expect the new pnpm dlx gentle-engram@X pi-engram init form instead of the old pnpm dlx --package ... form.

Your branch still expects the old form, so the CI run uses the pre-#660 assertion and fails.

Fix: rebase against current main — the conflict should resolve cleanly (your changes are in different files than what #660 touched, but the integration test now has the new expectation). After the rebase, this specific failure goes away.

Pseudocode:

git fetch upstream main
git rebase upstream/main
# resolve any conflicts (likely none for this PR's scope)
git push --force-with-lease

The other PR you have open (and #371 + #374 + #372) all show the same failure for the same reason — same rebase fixes all three. Thanks for sticking with these.

The SKILL.md template and the Frontmatter Fields table instructed every
generated skill to set metadata.author to 'gentleman-programming',
so skills created by any user ended up attributed to the wrong author.

Replace the template value with a '{your-github-username}' placeholder
and update the docs row to instruct the AI to infer the author from
'git config user.name' or ask the user — never hardcode.

The skill-creator's own frontmatter author is intentionally kept.

Closes Gentleman-Programming#98
@Basparin Basparin force-pushed the fix/98-skill-creator-author-placeholder branch from 6f646f7 to b665be2 Compare June 10, 2026 13:30
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR updates the skill-creator template documentation to use a placeholder for the author field instead of a hardcoded example name, allowing users to substitute their own GitHub username when creating new skills.

Changes

Template Placeholder Update

Layer / File(s) Summary
Frontmatter author placeholder
internal/assets/skills/skill-creator/SKILL.md
The metadata.author field in the skill-creator frontmatter template is changed from the fixed example value "gentleman-programming" to the placeholder "{your-github-username}".

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR addresses issue #98 by replacing the hardcoded author with a placeholder in the SKILL.md template. Golden snapshot files were excluded by path filters and cannot be verified. Golden snapshot files in testdata/ are excluded from review but mentioned as updated in PR objectives; verify they properly reflect the author placeholder change in the excluded test files.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and concisely describes the primary change: replacing hardcoded author in the skill-creator template and docs.
Out of Scope Changes check ✅ Passed The PR focuses narrowly on fixing the hardcoded author issue in skill-creator; all changes directly address issue #98 with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@Basparin

Copy link
Copy Markdown
Contributor Author

@Alan-TheGentleman rebaseado sobre main actual (03457e9). Main reescribió el SKILL.md del skill-creator otra vez y el template inline volvió a traer author: gentleman-programming hardcodeado — el fix se re-aplica a la línea nueva. Diff final: 5 files, 1 línea c/u (template + 4 goldens regenerados). TestGoldenSkills_* 4/4 PASS local. Único pendiente de mi lado: nada — falta solo el label type:bug que no puedo auto-aplicarme.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/assets/skills/skill-creator/SKILL.md (1)

53-64: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider adding explicit guidance for populating the author field.

The template now correctly uses a placeholder for metadata.author, but the document doesn't instruct the AI how to populate it. Without guidance, an AI agent might leave the placeholder as-is or use inconsistent heuristics.

Consider adding an instruction in the execution steps, for example after line 64 or in the inline fallback rules (around line 72):

- Populate `metadata.author` by inferring from `git config user.name` or prompting the user; never leave the placeholder or hardcode a value.

This would align with the PR objectives and ensure consistent, correct author attribution in generated skills.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/assets/skills/skill-creator/SKILL.md` around lines 53 - 64, Update
the SKILL.md frontmatter guidance to explicitly instruct how to populate
metadata.author: in the execution steps (after the frontmatter example) or in
the inline fallback rules, add a rule that metadata.author must be populated by
inferring the author from git config user.name or by prompting the user for
their GitHub username, and that the generator must never leave the placeholder
or hardcode a value; reference the frontmatter example (the
name/description/license/metadata block) and the metadata.author field when
adding this instruction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@internal/assets/skills/skill-creator/SKILL.md`:
- Around line 53-64: Update the SKILL.md frontmatter guidance to explicitly
instruct how to populate metadata.author: in the execution steps (after the
frontmatter example) or in the inline fallback rules, add a rule that
metadata.author must be populated by inferring the author from git config
user.name or by prompting the user for their GitHub username, and that the
generator must never leave the placeholder or hardcode a value; reference the
frontmatter example (the name/description/license/metadata block) and the
metadata.author field when adding this instruction.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1fe79a7f-9e47-428e-bec3-383d65cfd9d2

📥 Commits

Reviewing files that changed from the base of the PR and between 03457e9 and b665be2.

⛔ Files ignored due to path filters (4)
  • testdata/golden/skills-claude-skill-creator.golden is excluded by !testdata/**
  • testdata/golden/skills-kiro-skill-creator.golden is excluded by !testdata/**
  • testdata/golden/skills-opencode-skill-creator.golden is excluded by !testdata/**
  • testdata/golden/skills-windsurf-skill-creator.golden is excluded by !testdata/**
📒 Files selected for processing (1)
  • internal/assets/skills/skill-creator/SKILL.md

@Alan-TheGentleman Alan-TheGentleman merged commit c23d382 into Gentleman-Programming:main Jun 10, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skill-creator hardcodes author as 'gentleman-programming' in all user-created skills

2 participants