Skip to content

Fix two bugs#1

Open
embeddedt wants to merge 4 commits into
joelhooks:mainfrom
embeddedt:fix-interpolation-bugs
Open

Fix two bugs#1
embeddedt wants to merge 4 commits into
joelhooks:mainfrom
embeddedt:fix-interpolation-bugs

Conversation

@embeddedt

@embeddedt embeddedt commented Apr 29, 2026

Copy link
Copy Markdown

Thanks for this extension. I encountered two issues while using it with a skill I originally built for Claude Code:

  • CC executes the interpolated command from the project's working directory, while the extension was previously running it from the skill folder.
  • The read hook didn't detect frontmatter correctly when the model chooses to do a partial read.

This PR fixes both issues.

In the interest of full disclosure, the code changes were done by GPT-5.4 Mini (with review from me).

Summary by CodeRabbit

  • Bug Fixes
    • Skill discovery and execution now correctly respect the project working directory.
    • Skill metadata is read from the full skill file on disk (not from snippets), with improved error handling.
    • Interpolation now reliably resolves read targets against the project directory and provides the skill directory info to execution environments.

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2b60e984-78ef-4e54-90fc-a68137971eed

📥 Commits

Reviewing files that changed from the base of the PR and between 19fe565 and 7f6458b.

📒 Files selected for processing (1)
  • index.ts

📝 Walkthrough

Walkthrough

The extension now uses ctx.cwd as the working directory for skill discovery, interpolation execution, and resolving read targets. The /skill: input handler and tool_result handler re-read full SKILL.md from disk (resolved against ctx.cwd) to parse frontmatter and decide Bash interpolation; CLAUDE_SKILL_DIR may be set in the exec environment.

Changes

Cohort / File(s) Summary
Core handler
index.ts
Threaded ctx.cwd through /skill: processing and tool_result handling; replaced process.cwd()/dirname() usage with ctx.cwd; resolved read targets against ctx.cwd; re-read full SKILL.md from disk for frontmatter/allowed-tools checks; conditional injection of CLAUDE_SKILL_DIR into execSync env; added error handling around disk reads.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Extension
    participant FS as File System
    participant Shell as Shell/exec
    Client->>Extension: /skill: request (with ctx.cwd)
    Extension->>FS: discover SKILL.md using ctx.cwd
    FS-->>Extension: SKILL.md path + contents
    Extension->>Shell: run interpolation in ctx.cwd (env may include CLAUDE_SKILL_DIR)
    Shell-->>Extension: interpolation output
    Extension-->>Client: response
Loading
sequenceDiagram
    participant Tool as Read Tool
    participant Extension
    participant FS as File System
    participant Parser
    Tool->>Extension: returns read target path
    Extension->>FS: resolve path against ctx.cwd and read full SKILL.md
    FS-->>Extension: file contents or error
    Extension->>Parser: parse frontmatter / allowed-tools
    alt allowed
        Extension->>Shell: run Bash interpolation in ctx.cwd
        Shell-->>Extension: result
    else not allowed
        Extension-->>Tool: skip interpolation
    end
    Extension-->>Tool: final processed output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniff the cwd where skills abide,
I read the files with careful pride,
I set a path and hop to run,
CLAUDE_SKILL_DIR shines like sun —
A tiny hop, a tidy stride.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix two bugs' is vague and non-descriptive, lacking specific detail about what bugs are being fixed or which components are affected. Consider a more specific title like 'Fix interpolation working directory and frontmatter detection' to clearly convey the nature of the bugs being addressed.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

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

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
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

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

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

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 `@index.ts`:
- Line 4: Resolve the tool's input path against the execution cwd before
re-reading the file: compute a resolvedPath = path.resolve(ctx.cwd,
event.input.path) (add resolve to the import if missing) and use that
resolvedPath in the readFileSync call instead of event.input.path so the
frontmatter check reads the correct file when process.cwd() differs from
ctx.cwd; ensure any later uses that re-read or reference the same file (e.g.,
where readFileSync is called and any subsequent file operations tied to
event.input.path) use resolvedPath for consistency.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: f682f5e9-2e5c-45bc-8060-f95dc1732bac

📥 Commits

Reviewing files that changed from the base of the PR and between 94b0580 and 19fe565.

📒 Files selected for processing (1)
  • index.ts

Comment thread index.ts Outdated
@embeddedt

Copy link
Copy Markdown
Author

I decided to upstream another change to set CLAUDE_SKILL_DIR as well, as otherwise referencing other files in the skill from the command is a bit difficult. I don't like using a vendor-specific name but I don't think there's a standard for that variable yet.

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