fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(init): handle skill installer failures and stub in tests - #116

Merged
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer
Apr 7, 2026
Merged

fix(init): handle skill installer failures and stub in tests#116
wyattjoh merged 2 commits into
mainfrom
fix/init-skills-installer

Conversation

@wyattjoh

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the Bun.spawn(["npx", "skills", "add", ...]) call inside installSkills in try/catch so a missing npx binary on a stripped-down CI image can't crash init after the scaffold has already succeeded.
  • Add an installSkills spy to init/index.test.ts. Without it, running the unit test would shell out to the real npx skills add and pollute the working tree with .augment/, .claude/, skills/clerk-*, etc. on every test run.
  • Document the existing --no-skills flag in both the root README and init/README.md.

Test plan

  • bun run test passes (57 passed)
  • bun run format and bun run lint (cli-core) clean
  • Manual: run bun test packages/cli-core/src/commands/init/index.test.ts and confirm no stray skills/clerk-* directories are written

@coderabbitai

coderabbitaiBot commented Apr 7, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6a84bfad-2c45-4a65-97b5-a2088163bdaf

📥 Commits

Reviewing files that changed from the base of the PR and between dd71548 and 0e67ab9.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/init/index.test.ts

📝 Walkthrough

Walkthrough

Adds a --no-skills option to clerk init and documents an optional “agent skills install” step in README files. The init examples and usage/options were updated to show --no-skills. Tests now spy on installSkills to avoid performing real installs during test runs. installSkills wraps the spawn call in a try/catch to catch synchronous spawn errors (e.g., missing npx) and logs a warning instead of failing; non-zero exit codes still log a warning.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately captures the main changes: handling skill installer failures with try/catch and adding test stubs for the installSkills function.
Description check✅ PassedThe description clearly relates to the changeset, detailing the three main changes: wrapping spawn in try/catch, adding test spy, and documenting the --no-skills flag.

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


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

@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 41786f9 to 6cf5261CompareApril 7, 2026 17:32

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/commands/init/README.md`:
- Line 159: The README line about "Agent mode" is incorrect; update the
documentation to match the actual init flow where agent mode triggers the early
return and skips skills installation. Change the sentence to state that when
isAgent() is true (or when options.prompt is set) the init handler in
packages/cli-core/src/commands/init/index.ts returns early (see the if
(options.prompt || isAgent()) { ... return; } branch) and therefore skills are
not installed non-interactively unless a different code path explicitly runs the
installer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d5781c63-d2ba-47ce-aaff-db7a63aee5eb

📥 Commits

Reviewing files that changed from the base of the PR and between 41786f9 and 6cf5261.

📒 Files selected for processing (4)
  • README.md
  • packages/cli-core/src/commands/init/README.md
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts
✅ Files skipped from review due to trivial changes (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/cli-core/src/commands/init/index.test.ts
  • packages/cli-core/src/commands/init/skills.ts

Comment threadpackages/cli-core/src/commands/init/README.md Outdated
Wraps the `Bun.spawn(["npx", "skills", "add", ...])` call inside
`installSkills` in try/catch so a missing `npx` binary on a stripped-down
CI image can't crash init *after* the scaffold has already succeeded.
Adds an `installSkills` spy to init/index.test.ts. Without it, running
the unit test would shell out to the real `npx skills add` and pollute
the working tree with `.augment/`, `.claude/`, `skills/clerk`, etc. on
every test run.
Documents the existing `--no-skills` flag in both READMEs.
@wyattjoh
wyattjohforce-pushed the fix/init-skills-installer branch from 6cf5261 to dd71548CompareApril 7, 2026 17:41
Comment threadpackages/cli-core/src/commands/init/index.test.ts Outdated
Co-authored-by: Rafael Thayto <rafa.thayto@gmail.com>
@wyattjoh
wyattjoh merged commit f1f8e90 into mainApr 7, 2026
2 of 3 checks passed
@wyattjoh
wyattjoh deleted the fix/init-skills-installer branch April 7, 2026 18:16
Sign up for freeto 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.

3 participants

@wyattjoh@rafa-thayto@Railly