feat(skills): detect installed AI agents and install the DeployHQ skill#26
Open
facundofarias wants to merge 5 commits into
Open
feat(skills): detect installed AI agents and install the DeployHQ skill#26facundofarias wants to merge 5 commits into
facundofarias wants to merge 5 commits into
Conversation
Add a Wrangler-style post-login hook and a standalone `dhq skills`
command that detect AI coding agents on this machine and install the
DeployHQ skill (skills/deployhq/) into each agent's native format.
Twelve targets supported:
- Aider (user-scope, owned file + wire-up note)
- Antigravity (project-scope, repo-root AGENTS.md)
- Claude Code (user-scope, ~/.claude/skills/ tree)
- Cline (project-scope, .clinerules/deployhq.md)
- Codex CLI (user-scope, ~/.codex/AGENTS.md sentinel section)
- Continue.dev (user-scope, ~/.continue/rules/deployhq.md)
- Cursor (user-scope, ~/.cursor/rules/deployhq.mdc)
- Gemini CLI (user-scope, ~/.gemini/GEMINI.md sentinel section)
- GitHub Copilot (project-scope, .github/copilot-instructions.md)
- Kiro CLI (project-scope, .kiro/steering/deployhq.md)
- OpenCode (user-scope, $XDG_CONFIG_HOME/opencode/AGENTS.md)
- Windsurf (user-scope, ~/.codeium/windsurf/memories/global_rules.md)
Key design choices:
- Scope (User vs Project) is first-class on the Target interface; the
hello prompt only auto-offers ScopeUser targets so we never mutate
a user's repo as a side effect of logging in.
- Targets that share an instructions file with the user (Windsurf,
Codex, Gemini, OpenCode, Copilot, Antigravity) own only a
sentinel-bounded section; user content outside the markers is
preserved byte-for-byte across (re)installs.
- Targets that own their file entirely (Aider, Cline, Continue) use
a top-of-file `<!-- deployhq-skill v1 -->` HTML comment for
versioning.
- All installs are idempotent: install -> re-install produces
byte-identical files. Outdated installs upgrade silently.
- An optional Noter interface lets Aider surface the one-line
`~/.aider.conf.yml` wire-up step we explicitly refuse to automate.
The runtime agent (per harness.Detect) is auto-installed during
`dhq hello` without prompting — if the user is running dhq from
inside Claude Code right now, they want the skill. Other user-scope
agents detected on disk are batched into a single Y/n prompt.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds a complete ChangesAgent Skills Subsystem
Sequence Diagram(s)sequenceDiagram
actor User
participant hello as dhq hello
participant offer as offerSkillInstall
participant detect as skillinstaller.DetectInstalled()
participant harness as harness.Detect()
participant prompt as Y/n Prompt
participant install as t.Install()
rect rgba(70, 130, 180, 0.5)
Note over hello,install: Step 2 — non-fatal skill install
User->>hello: dhq hello
hello->>offer: offerSkillInstall(env)
offer->>detect: DetectInstalled()
detect-->>offer: []DetectResult
offer->>harness: Detect() for runtime agent
harness-->>offer: runtime agent or nil
offer->>install: auto-install runtime (no prompt, even NonInteractive)
install-->>offer: path or error (warning only)
offer->>prompt: Confirm install for N other user-scope agents? [Y/n]
prompt-->>offer: yes / no / skip
offer->>install: installOne() per remaining if confirmed
install-->>offer: path + optional PostInstallNote
offer-->>hello: nil (non-fatal)
end
rect rgba(60, 179, 113, 0.5)
Note over User,install: dhq skills install command
User->>hello: dhq skills install --agent cursor
hello->>detect: Find("cursor")
detect-->>hello: cursor Target
hello->>install: cursor.Install()
install-->>hello: ~/.cursor/rules/deployhq.mdc
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d55b132992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+78
to
+80
| for _, d := range skillinstaller.DetectInstalled() { | ||
| targets = append(targets, d.Target) | ||
| } |
There was a problem hiding this comment.
When dhq skills install is run without --agent inside any git repository, DetectInstalled() includes project-scoped targets whose detection is just “there is a .git ancestor” (Copilot, Antigravity, Cline, Kiro, etc.). This loop then installs all of them and mutates repo files such as .github/, AGENTS.md, .clinerules, and .kiro/, even though the ScopeProject contract says those targets are explicit --agent opt-in. Please filter ScopeProject here or prompt before adding them to the bulk install list.
Useful? React with 👍 / 👎.
| if err != nil { | ||
| return "", err | ||
| } | ||
| return cwd, nil |
There was a problem hiding this comment.
When dhq is invoked from a subdirectory, inRepo() accepts the ancestor .git, but this helper still returns the subdirectory as the root. Detect() and Install() then read/write subdir/.github/copilot-instructions.md and subdir/.github/copilot/deployhq instead of the repository root, so Copilot will not load the installed instructions. Walk up and return the directory containing .git for this project-scoped target.
Useful? React with 👍 / 👎.
…epo root Codex review (PR #26) flagged two bugs in the skill-installer: 1. `dhq skills install` (no --agent) iterated every detected target, silently mutating .github/, AGENTS.md, .clinerules/, and .kiro/ in the user's current repo. That contradicts the PR's own contract that project-scope targets are opt-in. Filter bulk install to ScopeUser; print a hint listing skipped project-scope agents and the --agent invocation to install each. 2. Copilot's repoRoot() returned cwd, not the .git-bearing ancestor. Running `dhq` from a subdirectory wrote to `subdir/.github/copilot-instructions.md`, where Copilot doesn't look. Same pattern existed in Cline, Kiro, and Antigravity. Extract a shared findRepoRoot() helper and route all four project-scope targets through it for both Detect() and Install(). Adds regression tests in repo_test.go: each project-scope target, when invoked from a nested subdirectory, must write at the repo root and must NOT create anything in the subdirectory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
internal/skillinstaller/section_test.go (1)
58-65: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueOptional: simplify contains() helper with strings.Contains.
The
contains()helper implements substring search manually (lines 58-65). Go's standard library providesstrings.Contains()which is more idiomatic and equally efficient here:func contains(s, sub string) bool { return strings.Contains(s, sub) }Or inline it in the test calls. This is purely a style preference.
🤖 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/skillinstaller/section_test.go` around lines 58 - 65, The `contains()` helper function manually implements substring search which is unnecessary since Go's standard library provides this functionality. Replace the manual implementation of the `contains()` function with a simple call to `strings.Contains()`, which is more idiomatic and efficient. Alternatively, you can inline `strings.Contains()` directly at the call sites in the test instead of keeping a separate helper function. Make sure to import the strings package if you choose this approach.internal/commands/skills.go (1)
37-39: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAlign
skills listwording with actual output.Line 38 says “detected AI agents,” but Lines 43-49 enumerate all registered targets (including
not-installed). Either switch to detected-only rows or update command/help text to “supported AI agents and status”.Also applies to: 43-49
🤖 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/commands/skills.go` around lines 37 - 39, The Short description for the list subcommand states "detected AI agents" but the actual output enumeration includes all registered targets including not-installed ones. Update the Short field text to say "supported AI agents and status" instead to accurately reflect the actual output displayed in the command implementation, ensuring the help text matches what users will see when running the command.internal/skillinstaller/copilot_test.go (1)
180-191: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen ancestor test to verify install location, not just detect status.
This case proves nested repo detection, but it does not assert that
Install()writes to<repo>/.github/copilot-instructions.md. Adding that assertion will prevent regressions where nested cwd writes to the wrong directory.Suggested test extension
func TestCopilot_InRepo_FindsAncestor(t *testing.T) { @@ withCwd(t, sub) if got := (copilot{}).Detect(); got != StatusAvailable { t.Fatalf("Detect() nested in repo = %v, want StatusAvailable", got) } + + gotPath, err := (copilot{}).Install() + if err != nil { + t.Fatalf("Install() nested in repo = %v", err) + } + wantPath := filepath.Join(root, copilotInstructionsFile) + if gotPath != wantPath { + t.Fatalf("Install() path = %q, want %q", gotPath, wantPath) + } }🤖 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/skillinstaller/copilot_test.go` around lines 180 - 191, The TestCopilot_InRepo_FindsAncestor test function currently only verifies that Detect() returns StatusAvailable when called from a nested directory, but does not assert that Install() writes the copilot-instructions.md file to the correct repository root location. After the existing Detect() assertion, add a call to Install() on the copilot struct and then verify that the copilot-instructions.md file exists at the expected path within the repository root's .github directory (constructed from the root variable). This ensures that when installing from a nested working directory, the file is written to the correct ancestor repository location and not to the nested cwd.
🤖 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.
Inline comments:
In `@internal/commands/skills.go`:
- Around line 78-80: The code in the DetectInstalled loop unconditionally
includes all detected targets without filtering by scope, allowing
project-scoped installers to run without explicit opt-in when dhq skills install
is called without the --agent flag. Filter the targets returned by
skillinstaller.DetectInstalled() to only include those appropriate for the
current scope context. Specifically, exclude project-scoped targets (check the
d.Target scope property) unless the command was invoked with explicit opt-in
such as the --agent flag, ensuring project-scope modifications require
intentional user action.
In `@internal/skillinstaller/aider_test.go`:
- Around line 181-183: The test case with the label "v42 mid file" on line 182
is validating that a marker found in the middle of a file (with content "x\n"
before the <!-- deployhq-skill v42 --> marker) is detected as a valid version.
This conflicts with the contract that owned-file markers must be at the top of
the file. Remove this test case entirely, as it codifies invalid behavior that
can cause false positives in the Detect() function when processing non-owned
files.
In `@internal/skillinstaller/aider.go`:
- Around line 124-127: The path variable p is being interpolated directly into
YAML and CLI syntax within the fmt.Sprintf call without proper quoting, which
will break on Windows paths containing backslashes or paths with spaces. Add
appropriate quote characters around the %s placeholders in the format string to
properly escape the path values in both the YAML snippet (read: [%s]) and the
CLI argument (--read %s) portions of the returned string.
In `@internal/skillinstaller/antigravity.go`:
- Around line 61-70: The Detect method uses the current working directory (cwd)
for file operations instead of the repository root, which causes files to be
installed in the wrong location when running from a subdirectory. Modify the
antigravity Detect method to resolve and use the repository root path (obtained
via the same mechanism that inRepo() uses to detect .git) instead of the raw cwd
when reading files like antigravityInstructionsFile at line 69. Apply this same
fix to the other file operations at lines 94 and 102 that also use cwd to ensure
all read/write operations are consistently resolved from the repository root
rather than the current working directory.
In `@internal/skillinstaller/cline.go`:
- Around line 56-74: The skillFile() and rulesPathStat() methods construct paths
to .clinerules using getCwd() instead of the repository root, causing them to
target incorrect locations from nested directories. Replace getCwd() calls in
both the skillFile() method (lines 57-63) and rulesPathStat() method (lines
69-73) with logic that finds the repository root by walking up the directory
tree to locate the .git directory. This ensures the methods consistently resolve
.clinerules paths relative to the actual repository root rather than the current
working directory.
In `@internal/skillinstaller/copilot.go`:
- Around line 49-55: The repoRoot() method is returning the current working
directory directly without resolving to the actual repository root, which causes
Copilot files to be placed in incorrect subdirectories when invoked from
repo/subdir. Modify the repoRoot() function to traverse up the directory tree
from the current working directory to find and return the ancestor directory
containing .git, ensuring that methods at lines 89, 113, and 121 place files
correctly at the repository root rather than in subdirectories.
In `@internal/skillinstaller/installer.go`:
- Around line 133-138: The DetectInstalled() docstring incorrectly describes
special handling for the runtime agent, claiming it is included with special
logic even if Status would be StatusInstalled. However, the actual
implementation simply filters out StatusNotInstalled and returns all other
statuses without any runtime agent-specific logic. Update the docstring to
accurately state that DetectInstalled() returns all agents that are installed
(all statuses except StatusNotInstalled), and clarify that the decision to
handle the runtime agent differently (e.g., for silent upgrades) is delegated to
the caller.
- Around line 149-158: The Find() function has conflicting behavior between its
docstring and implementation. The docstring on line 150 claims "Matching is
case-insensitive on the canonical name," but the comparison at lines 152-156
uses case-sensitive equality check (t.Name() == name). Verify the intended
behavior by checking how Find() is used throughout the codebase, then either
update the docstring to remove the "case-insensitive" claim if case-sensitive
matching is correct, or implement case-insensitive comparison by converting both
t.Name() and name to lowercase before comparing them in the Find() function.
In `@internal/skillinstaller/kiro.go`:
- Around line 55-61: The skillPath() method in the kiro type uses getCwd() to
build the path to the skill file, but this returns the current working directory
rather than anchoring to the repository root. When kiro tools are executed from
nested directories within a repo, this causes the skill file to be detected or
installed in the wrong location. Replace the getCwd() call in skillPath() with a
function that determines and returns the repository root directory, then build
the path from that root. Also review and apply the same fix to other methods in
the kiro type that are mentioned to be affected (lines 63-106) to ensure
consistent behavior across all file path operations.
In `@internal/skillinstaller/opencode.go`:
- Around line 33-36: The xdgConfigDir() function accepts any non-empty
XDG_CONFIG_HOME environment variable value without validating that it is an
absolute path. Add a check after retrieving the XDG_CONFIG_HOME environment
variable to ensure the path is absolute before returning it; if the path is
relative, skip it and fall through to the default behavior. Apply the same
validation to the other similar functions mentioned in the comment that handle
environment variable paths (those around lines 44-50 and 75-104) to ensure all
config and data directory lookups reject relative paths and prevent unintended
writes to the current directory.
In `@internal/skillinstaller/section.go`:
- Around line 45-58: The mergeSection function calls joinAround on lines that
construct the return value, but the joinAround function is not defined in this
file and no import provides it, causing a compilation error. Either define the
joinAround helper function directly in section.go (it should accept three string
parameters: pre, section, and post, and return a joined string) or verify that
joinAround exists elsewhere in the skillinstaller package and is being called
with the correct qualified name. Ensure the function is accessible to
mergeSection when it makes the two joinAround calls.
---
Nitpick comments:
In `@internal/commands/skills.go`:
- Around line 37-39: The Short description for the list subcommand states
"detected AI agents" but the actual output enumeration includes all registered
targets including not-installed ones. Update the Short field text to say
"supported AI agents and status" instead to accurately reflect the actual output
displayed in the command implementation, ensuring the help text matches what
users will see when running the command.
In `@internal/skillinstaller/copilot_test.go`:
- Around line 180-191: The TestCopilot_InRepo_FindsAncestor test function
currently only verifies that Detect() returns StatusAvailable when called from a
nested directory, but does not assert that Install() writes the
copilot-instructions.md file to the correct repository root location. After the
existing Detect() assertion, add a call to Install() on the copilot struct and
then verify that the copilot-instructions.md file exists at the expected path
within the repository root's .github directory (constructed from the root
variable). This ensures that when installing from a nested working directory,
the file is written to the correct ancestor repository location and not to the
nested cwd.
In `@internal/skillinstaller/section_test.go`:
- Around line 58-65: The `contains()` helper function manually implements
substring search which is unnecessary since Go's standard library provides this
functionality. Replace the manual implementation of the `contains()` function
with a simple call to `strings.Contains()`, which is more idiomatic and
efficient. Alternatively, you can inline `strings.Contains()` directly at the
call sites in the test instead of keeping a separate helper function. Make sure
to import the strings package if you choose this approach.
🪄 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: 3837e22c-155a-48c5-ba7d-8145e7509dc3
📒 Files selected for processing (34)
internal/commands/hello.gointernal/commands/hello_skills.gointernal/commands/root.gointernal/commands/skills.gointernal/skillinstaller/aider.gointernal/skillinstaller/aider_test.gointernal/skillinstaller/antigravity.gointernal/skillinstaller/antigravity_test.gointernal/skillinstaller/claude.gointernal/skillinstaller/cline.gointernal/skillinstaller/cline_test.gointernal/skillinstaller/codex.gointernal/skillinstaller/codex_test.gointernal/skillinstaller/continue.gointernal/skillinstaller/continue_test.gointernal/skillinstaller/copilot.gointernal/skillinstaller/copilot_test.gointernal/skillinstaller/cursor.gointernal/skillinstaller/cursor_test.gointernal/skillinstaller/flatten.gointernal/skillinstaller/frontmatter.gointernal/skillinstaller/gemini.gointernal/skillinstaller/gemini_test.gointernal/skillinstaller/installer.gointernal/skillinstaller/installer_test.gointernal/skillinstaller/kiro.gointernal/skillinstaller/kiro_test.gointernal/skillinstaller/opencode.gointernal/skillinstaller/opencode_test.gointernal/skillinstaller/section.gointernal/skillinstaller/section_test.gointernal/skillinstaller/windsurf.gointernal/skillinstaller/windsurf_test.goskills/embed.go
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/skillinstaller/repo.go (1)
18-22: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd test coverage for
getCwd()error case.The error path when
getCwd()fails (line 20-21) is not covered by tests. While this is straightforward error handling, testing it would improve coverage and prevent regressions.🧪 Suggested test case
func TestFindRepoRoot_GetCwdError(t *testing.T) { oldGetCwd := getCwd defer func() { getCwd = oldGetCwd }() getCwd = func() (string, error) { return "", os.ErrPermission } got, ok := findRepoRoot() if ok { t.Error("findRepoRoot() returned ok when getCwd failed") } if got != "" { t.Errorf("findRepoRoot() = %q, want empty string on error", got) } }🤖 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/skillinstaller/repo.go` around lines 18 - 22, Add a test case for the error path in the findRepoRoot() function when getCwd() fails. Create a new test function that temporarily replaces the getCwd function with a mock implementation that returns an error, then verify that findRepoRoot() returns an empty string and false (the ok boolean should be false). Use a defer statement to restore the original getCwd function after the test completes, ensuring the test properly isolates the getCwd() error scenario that is currently not covered by existing tests.internal/commands/skills.go (1)
93-98: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winError message could be misleading when only project-scope agents are detected.
If all detected agents are project-scope (e.g., only Copilot and Cline are installed), they're filtered into
skippedProjectandtargetsremains empty. The error "No supported AI agents detected on this machine" is then factually incorrect—agents were detected but excluded from bulk install.Consider a more accurate message when
len(skippedProject) > 0:if len(targets) == 0 { if len(skippedProject) > 0 { return &output.UserError{ Message: "No user-scope AI agents available for bulk install", Hint: "Install a user-scope agent (e.g. Claude Code) or pass --agent <name> to install for a project-scope agent.", } } return &output.UserError{ Message: "No supported AI agents detected on this machine", Hint: "Install one (e.g. Claude Code) and re-run, or pass --agent <name>.", } }🤖 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/commands/skills.go` around lines 93 - 98, The error message returned when len(targets) == 0 is misleading when project-scope agents are detected but filtered into skippedProject. Modify the condition block starting at the len(targets) == 0 check to first verify if len(skippedProject) > 0. If true, return a different UserError with Message "No user-scope AI agents available for bulk install" and an appropriate Hint directing users to install user-scope agents or use the --agent flag. If skippedProject is empty, return the existing error about no supported agents being detected. This way, the error message accurately reflects whether agents were found but excluded versus no agents being detected at all.
🤖 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.
Nitpick comments:
In `@internal/commands/skills.go`:
- Around line 93-98: The error message returned when len(targets) == 0 is
misleading when project-scope agents are detected but filtered into
skippedProject. Modify the condition block starting at the len(targets) == 0
check to first verify if len(skippedProject) > 0. If true, return a different
UserError with Message "No user-scope AI agents available for bulk install" and
an appropriate Hint directing users to install user-scope agents or use the
--agent flag. If skippedProject is empty, return the existing error about no
supported agents being detected. This way, the error message accurately reflects
whether agents were found but excluded versus no agents being detected at all.
In `@internal/skillinstaller/repo.go`:
- Around line 18-22: Add a test case for the error path in the findRepoRoot()
function when getCwd() fails. Create a new test function that temporarily
replaces the getCwd function with a mock implementation that returns an error,
then verify that findRepoRoot() returns an empty string and false (the ok
boolean should be false). Use a defer statement to restore the original getCwd
function after the test completes, ensuring the test properly isolates the
getCwd() error scenario that is currently not covered by existing tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 740957bf-9e70-4777-84e6-991a6f88d286
📒 Files selected for processing (7)
internal/commands/skills.gointernal/skillinstaller/antigravity.gointernal/skillinstaller/cline.gointernal/skillinstaller/copilot.gointernal/skillinstaller/kiro.gointernal/skillinstaller/repo.gointernal/skillinstaller/repo_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/skillinstaller/kiro.go
- internal/skillinstaller/copilot.go
- internal/skillinstaller/cline.go
- internal/skillinstaller/antigravity.go
Addresses CodeRabbit findings on PR #26 that survived the first fixup: - aider: PostInstallNote double-quotes the path so the suggested `read: ["..."]` snippet and `--read "..."` invocation are safe to paste verbatim when the user's home contains spaces or other characters that would otherwise need shell/YAML escaping. - opencode: xdgConfigDir requires XDG_CONFIG_HOME to be absolute, per the XDG Base Directory spec. Falls through to ~/.config when the variable holds a relative path, so a misconfigured env never causes writes into the dev's cwd. - installer: tighten two docstrings that lied about behavior. DetectInstalled no longer claims runtime-agent special handling (that lives in offerSkillInstall, not here); Find documents its actual case-sensitive matching and explains why canonical names are always lowercase. - flatten: parseOwnedFileVersion now requires the marker to be the first thing in the body. Owned-file targets (Aider/Cline/Continue/ Kiro) write the marker at the top via buildOwnedSkillFile, so any content above it means the file isn't ours — Detect now reports Available so the next install rewrites cleanly. aider_test flips the corresponding case from "v42 mid file → 42" to "rejected". - skills list: Short text reads "List supported AI agents and their skill install status" — the output enumerates every registered target, not just those detected on disk. - section_test: drops the hand-rolled `contains` helper in favour of strings.Contains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- repo_test: cover the getCwd-error path in findRepoRoot so a future refactor that silently turns the error into "no repo found" is caught. - skills.go: when bulk install is invoked in a context where every detected agent is project-scope (e.g. inside a git repo with only Copilot/Cline/etc. installed), the previous "No supported AI agents detected on this machine" message was a lie. Distinguish the case and surface the exact `dhq skills install --agent <name>` line for each filtered agent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
internal/skillinstaller/cursor.go (1)
124-124: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winQuote the frontmatter description scalar for YAML safety.
Line 124 writes an unquoted YAML scalar; descriptions containing characters like
:or#can break parsing or truncate content.Proposed fix
- fmt.Fprintf(&buf, "description: %s\n", oneLine(description)) + fmt.Fprintf(&buf, "description: %q\n", oneLine(description))🤖 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/skillinstaller/cursor.go` at line 124, The fmt.Fprintf call writing the YAML description field is outputting an unquoted scalar value, which can break YAML parsing if the description contains special characters like colons or hashes. Fix this by modifying the format string in the fprintf call to wrap the description value in quotes, ensuring it is treated as a safe YAML scalar string regardless of its content.internal/skillinstaller/opencode_test.go (1)
125-146: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd a regression case for relative
XDG_CONFIG_HOME.
TestXDGConfigDircurrently covers empty and absolute values only. A relative-value assertion would lock in the safety behavior and prevent regression to cwd-relative writes.Suggested patch
func TestXDGConfigDir(t *testing.T) { home := t.TempDir() withHomeDir(t, home) @@ if got != "/some/path" { t.Errorf("XDG override = %q, want %q", got, "/some/path") } + + t.Setenv("XDG_CONFIG_HOME", "relative/path") + got, err = xdgConfigDir() + if err != nil { + t.Fatal(err) + } + if got != filepath.Join(home, ".config") { + t.Errorf("relative XDG should fall back = %q, want %q", got, filepath.Join(home, ".config")) + } }🤖 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/skillinstaller/opencode_test.go` around lines 125 - 146, In the TestXDGConfigDir function, add a third test case after the existing absolute path check to verify the behavior when XDG_CONFIG_HOME is set to a relative path value (e.g., "relative/path"). Use t.Setenv to set XDG_CONFIG_HOME to a relative path, call xdgConfigDir(), and assert the expected behavior to prevent regression and ensure the function safely handles relative paths rather than writing to cwd-relative locations.
🤖 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.
Inline comments:
In `@internal/commands/skills.go`:
- Around line 18-21: The example text for the bulk "dhq skills install" command
(without the --agent flag) currently claims it "Install for every detected
agent," but the actual implementation intentionally skips project-scope targets.
Update this example text in the Examples section to accurately reflect what bulk
install actually does, making it clear to users which agent scopes are included
or excluded to avoid confusion about the command's repo mutation behavior.
In `@internal/skillinstaller/aider_test.go`:
- Around line 148-151: The test assertion around line 149 is comparing the note
against a raw filesystem path in the `want` variable, but the actual
post-install note contains a quoted or escaped version of the path. Fix this by
modifying the `want` variable to include the proper quoting/escaping that
matches the format the note actually outputs, so the test correctly validates
the path on all platforms including Windows. Reference the string comparison in
the if block where strings.Contains checks the note against want.
In `@internal/skillinstaller/aider.go`:
- Around line 130-145: The function quotePathForYAMLAndShell escapes the path
for both YAML and shell contexts using the same logic, but shell double-quoted
strings still evaluate $() and backticks, making the note's "safe to paste"
guarantee incorrect for edge-case paths. Create two separate functions: one for
YAML escaping (quotePathForYAML) and one for shell escaping (quotePathForShell).
The shell version should additionally escape dollar signs and backticks beyond
what the YAML version does. Then in the return statement of the parent function,
use quotePathForYAML for the YAML config example and quotePathForShell for the
shell command example.
In `@internal/skillinstaller/codex.go`:
- Around line 58-62: The Detect() function in codex.go currently treats all
errors from os.ReadFile() as StatusAvailable, but permission and I/O errors
should be handled separately from missing files. Modify the error handling in
the Detect() function to use os.IsNotExist() to check if the error is
specifically a file-not-found error, and only return StatusAvailable in that
case. For other error types (permission denied, I/O errors, etc.), handle them
appropriately by either returning the error or a different status that indicates
failure rather than availability.
In `@internal/skillinstaller/flatten.go`:
- Around line 102-113: The loop iterating through entries in the references
directory currently processes all files, but the function contract specifies
only markdown files should be ingested. Add a check after the directory skip
condition to ensure only files with the .md extension are processed, filtering
out any non-markdown files before reading them with fs.ReadFile and adding them
to the output buffer.
In `@internal/skillinstaller/gemini.go`:
- Around line 51-54: The Detect() function currently returns StatusAvailable for
any error encountered when reading the geminiInstructionsFile, but this masks
actual read failures like permission errors. Modify the error handling in the
os.ReadFile() call to check if the error is specifically a file-not-found error
using os.IsNotExist(err), and only return StatusAvailable in that case. Other
read errors should be handled separately to properly reflect the actual
availability status rather than treating all errors the same way.
In `@internal/skillinstaller/opencode.go`:
- Around line 66-69: The Detect() function in the opencode.go file currently
treats all errors from os.ReadFile when reading the opencodeInstructionsFile as
StatusAvailable, which masks configuration problems. Instead, differentiate
between missing file errors and other read failures: check if the error returned
from os.ReadFile is specifically a "file not found" error using os.IsNotExist(),
and only return StatusAvailable for that case; for all other errors (permission
denied, broken config, etc.), return StatusUnavailable or a different
appropriate status to indicate the configuration state is inaccessible or
broken.
In `@internal/skillinstaller/windsurf.go`:
- Around line 73-77: The current code in the Detect() function treats all
os.ReadFile errors as StatusAvailable, but only file-not-found cases should map
there. Use os.IsNotExist(err) to distinguish between a missing rules file (which
should return StatusAvailable) and other types of errors like permission denied
or I/O errors (which should be handled differently). Modify the error handling
after the os.ReadFile(rulesPath) call to check the error type more precisely and
handle non-existence cases separately from other error conditions.
---
Nitpick comments:
In `@internal/skillinstaller/cursor.go`:
- Line 124: The fmt.Fprintf call writing the YAML description field is
outputting an unquoted scalar value, which can break YAML parsing if the
description contains special characters like colons or hashes. Fix this by
modifying the format string in the fprintf call to wrap the description value in
quotes, ensuring it is treated as a safe YAML scalar string regardless of its
content.
In `@internal/skillinstaller/opencode_test.go`:
- Around line 125-146: In the TestXDGConfigDir function, add a third test case
after the existing absolute path check to verify the behavior when
XDG_CONFIG_HOME is set to a relative path value (e.g., "relative/path"). Use
t.Setenv to set XDG_CONFIG_HOME to a relative path, call xdgConfigDir(), and
assert the expected behavior to prevent regression and ensure the function
safely handles relative paths rather than writing to cwd-relative locations.
🪄 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: 4be1bc79-3db9-4367-a419-1224c94d8d31
📒 Files selected for processing (36)
internal/commands/hello.gointernal/commands/hello_skills.gointernal/commands/root.gointernal/commands/skills.gointernal/skillinstaller/aider.gointernal/skillinstaller/aider_test.gointernal/skillinstaller/antigravity.gointernal/skillinstaller/antigravity_test.gointernal/skillinstaller/claude.gointernal/skillinstaller/cline.gointernal/skillinstaller/cline_test.gointernal/skillinstaller/codex.gointernal/skillinstaller/codex_test.gointernal/skillinstaller/continue.gointernal/skillinstaller/continue_test.gointernal/skillinstaller/copilot.gointernal/skillinstaller/copilot_test.gointernal/skillinstaller/cursor.gointernal/skillinstaller/cursor_test.gointernal/skillinstaller/flatten.gointernal/skillinstaller/frontmatter.gointernal/skillinstaller/gemini.gointernal/skillinstaller/gemini_test.gointernal/skillinstaller/installer.gointernal/skillinstaller/installer_test.gointernal/skillinstaller/kiro.gointernal/skillinstaller/kiro_test.gointernal/skillinstaller/opencode.gointernal/skillinstaller/opencode_test.gointernal/skillinstaller/repo.gointernal/skillinstaller/repo_test.gointernal/skillinstaller/section.gointernal/skillinstaller/section_test.gointernal/skillinstaller/windsurf.gointernal/skillinstaller/windsurf_test.goskills/embed.go
Comment on lines
+130
to
+145
| q := quotePathForYAMLAndShell(p) | ||
| return fmt.Sprintf( | ||
| "To load on every Aider run: add `read: [%s]` to ~/.aider.conf.yml "+ | ||
| "(or pass `--read %s` ad-hoc).", | ||
| q, q, | ||
| ) | ||
| } | ||
|
|
||
| // quotePathForYAMLAndShell wraps a path in double quotes with internal | ||
| // backslashes and double quotes escaped. The result is valid in both a | ||
| // YAML double-quoted scalar and a POSIX shell double-quoted string, which | ||
| // is the only quoting context the PostInstallNote needs to support. | ||
| func quotePathForYAMLAndShell(p string) string { | ||
| p = strings.ReplaceAll(p, `\`, `\\`) | ||
| p = strings.ReplaceAll(p, `"`, `\"`) | ||
| return `"` + p + `"` |
There was a problem hiding this comment.
Separate YAML quoting from shell quoting in the note.
Line 132 and Line 133 reuse one escaped string for both YAML and shell, but double-quoted shell arguments still evaluate $() and backticks. That makes the “safe to paste verbatim” guarantee incorrect for edge-case home paths.
Proposed fix
func (a aider) PostInstallNote() string {
p, err := a.skillPath()
if err != nil {
return ""
}
- q := quotePathForYAMLAndShell(p)
+ yamlPath := quotePathForYAML(p)
+ shellPath := quotePathForShell(p)
return fmt.Sprintf(
"To load on every Aider run: add `read: [%s]` to ~/.aider.conf.yml "+
"(or pass `--read %s` ad-hoc).",
- q, q,
+ yamlPath, shellPath,
)
}
-// quotePathForYAMLAndShell wraps a path in double quotes with internal
-// backslashes and double quotes escaped. The result is valid in both a
-// YAML double-quoted scalar and a POSIX shell double-quoted string, which
-// is the only quoting context the PostInstallNote needs to support.
-func quotePathForYAMLAndShell(p string) string {
+func quotePathForYAML(p string) string {
p = strings.ReplaceAll(p, `\`, `\\`)
p = strings.ReplaceAll(p, `"`, `\"`)
return `"` + p + `"`
}
+
+// quotePathForShell returns a single-quoted POSIX shell literal.
+func quotePathForShell(p string) string {
+ return `'` + strings.ReplaceAll(p, `'`, `'"'"'`) + `'`
+}🤖 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/skillinstaller/aider.go` around lines 130 - 145, The function
quotePathForYAMLAndShell escapes the path for both YAML and shell contexts using
the same logic, but shell double-quoted strings still evaluate $() and
backticks, making the note's "safe to paste" guarantee incorrect for edge-case
paths. Create two separate functions: one for YAML escaping (quotePathForYAML)
and one for shell escaping (quotePathForShell). The shell version should
additionally escape dollar signs and backticks beyond what the YAML version
does. Then in the return statement of the parent function, use quotePathForYAML
for the YAML config example and quotePathForShell for the shell command example.
Comment on lines
+58
to
+62
| data, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile)) | ||
| if err != nil { | ||
| // Codex installed but AGENTS.md not written yet — skill available. | ||
| return StatusAvailable | ||
| } |
There was a problem hiding this comment.
Handle unreadable AGENTS.md separately from missing file.
Detect() currently treats all read failures as StatusAvailable. Permission or I/O errors should not be classified as “available”, otherwise follow-up install flows can be prompted and then fail immediately.
Suggested patch
data, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile))
if err != nil {
- // Codex installed but AGENTS.md not written yet — skill available.
- return StatusAvailable
+ if os.IsNotExist(err) {
+ // Codex installed but AGENTS.md not written yet — skill available.
+ return StatusAvailable
+ }
+ return StatusNotInstalled
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile)) | |
| if err != nil { | |
| // Codex installed but AGENTS.md not written yet — skill available. | |
| return StatusAvailable | |
| } | |
| data, err := os.ReadFile(filepath.Join(cfg, codexAgentsFile)) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| // Codex installed but AGENTS.md not written yet — skill available. | |
| return StatusAvailable | |
| } | |
| return StatusNotInstalled | |
| } |
🤖 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/skillinstaller/codex.go` around lines 58 - 62, The Detect() function
in codex.go currently treats all errors from os.ReadFile() as StatusAvailable,
but permission and I/O errors should be handled separately from missing files.
Modify the error handling in the Detect() function to use os.IsNotExist() to
check if the error is specifically a file-not-found error, and only return
StatusAvailable in that case. For other error types (permission denied, I/O
errors, etc.), handle them appropriately by either returning the error or a
different status that indicates failure rather than availability.
Comment on lines
+51
to
+54
| data, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile)) | ||
| if err != nil { | ||
| return StatusAvailable | ||
| } |
There was a problem hiding this comment.
Differentiate “file missing” from other read errors in Detect().
Any read error currently returns StatusAvailable. Non-ENOENT failures should not be treated as available state.
Suggested patch
data, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile))
if err != nil {
- return StatusAvailable
+ if os.IsNotExist(err) {
+ return StatusAvailable
+ }
+ return StatusNotInstalled
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile)) | |
| if err != nil { | |
| return StatusAvailable | |
| } | |
| data, err := os.ReadFile(filepath.Join(cfg, geminiInstructionsFile)) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| return StatusAvailable | |
| } | |
| return StatusNotInstalled | |
| } |
🤖 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/skillinstaller/gemini.go` around lines 51 - 54, The Detect()
function currently returns StatusAvailable for any error encountered when
reading the geminiInstructionsFile, but this masks actual read failures like
permission errors. Modify the error handling in the os.ReadFile() call to check
if the error is specifically a file-not-found error using os.IsNotExist(err),
and only return StatusAvailable in that case. Other read errors should be
handled separately to properly reflect the actual availability status rather
than treating all errors the same way.
Comment on lines
+66
to
+69
| data, err := os.ReadFile(filepath.Join(cfg, opencodeInstructionsFile)) | ||
| if err != nil { | ||
| return StatusAvailable | ||
| } |
There was a problem hiding this comment.
Don’t map all instruction read failures to StatusAvailable.
Detect() should treat only missing files as available; other read failures indicate an inaccessible/broken config state.
Suggested patch
data, err := os.ReadFile(filepath.Join(cfg, opencodeInstructionsFile))
if err != nil {
- return StatusAvailable
+ if os.IsNotExist(err) {
+ return StatusAvailable
+ }
+ return StatusNotInstalled
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data, err := os.ReadFile(filepath.Join(cfg, opencodeInstructionsFile)) | |
| if err != nil { | |
| return StatusAvailable | |
| } | |
| data, err := os.ReadFile(filepath.Join(cfg, opencodeInstructionsFile)) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| return StatusAvailable | |
| } | |
| return StatusNotInstalled | |
| } |
🤖 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/skillinstaller/opencode.go` around lines 66 - 69, The Detect()
function in the opencode.go file currently treats all errors from os.ReadFile
when reading the opencodeInstructionsFile as StatusAvailable, which masks
configuration problems. Instead, differentiate between missing file errors and
other read failures: check if the error returned from os.ReadFile is
specifically a "file not found" error using os.IsNotExist(), and only return
StatusAvailable for that case; for all other errors (permission denied, broken
config, etc.), return StatusUnavailable or a different appropriate status to
indicate the configuration state is inaccessible or broken.
Comment on lines
+73
to
+77
| data, err := os.ReadFile(rulesPath) | ||
| if err != nil { | ||
| // Windsurf installed but no rules file yet — skill is available. | ||
| return StatusAvailable | ||
| } |
There was a problem hiding this comment.
Classify rules-file read errors more precisely in Detect().
The current branch treats all ReadFile errors as StatusAvailable. Only missing-file cases should map there.
Suggested patch
data, err := os.ReadFile(rulesPath)
if err != nil {
- // Windsurf installed but no rules file yet — skill is available.
- return StatusAvailable
+ if os.IsNotExist(err) {
+ // Windsurf installed but no rules file yet — skill is available.
+ return StatusAvailable
+ }
+ return StatusNotInstalled
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data, err := os.ReadFile(rulesPath) | |
| if err != nil { | |
| // Windsurf installed but no rules file yet — skill is available. | |
| return StatusAvailable | |
| } | |
| data, err := os.ReadFile(rulesPath) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| // Windsurf installed but no rules file yet — skill is available. | |
| return StatusAvailable | |
| } | |
| return StatusNotInstalled | |
| } |
🤖 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/skillinstaller/windsurf.go` around lines 73 - 77, The current code
in the Detect() function treats all os.ReadFile errors as StatusAvailable, but
only file-not-found cases should map there. Use os.IsNotExist(err) to
distinguish between a missing rules file (which should return StatusAvailable)
and other types of errors like permission denied or I/O errors (which should be
handled differently). Modify the error handling after the os.ReadFile(rulesPath)
call to check the error type more precisely and handle non-existence cases
separately from other error conditions.
Five clear wins from the second-round full review on PR #26: - skills.go: command Examples docstring claimed `dhq skills install` installs for "every detected agent" — out of date since the earlier fixup taught the bulk path to skip project-scope targets. Replace with examples that show the user-scope default plus explicit --agent invocations for project-scope agents. - flatten.go: references-tree loop now skips non-*.md entries. The embedded skill is markdown by contract, but a stray editor backup or future binary asset shouldn't get inlined verbatim into a single-file target's output. - cursor.go: write the .mdc description as a YAML-quoted scalar (`description: %q` instead of `%s`). Unquoted scalars containing `:` or `#` would corrupt the frontmatter; %q's Go-quoted form is also valid YAML double-quoted syntax. Test expectation updated to match the new quoted output. - opencode_test: add a regression case for relative XDG_CONFIG_HOME to lock in the spec-mandated fallback to ~/.config and prevent a future refactor from accidentally honouring relative paths and writing into the dev's cwd. - aider_test: the path-mention assertion now compares against the output of quotePathForYAMLAndShell rather than the raw path. Passed on Linux/macOS because the quoting is a no-op without metacharacters, but would have broken on Windows where backslashes in the path get escaped — making the substring search miss. Skipping by design: - Separating quotePathForYAML and quotePathForShell — paths with $/backtick in $HOME aren't realistic and the note is informational. - Replacing `if err != nil → StatusAvailable` with IsNotExist checks across Detect functions — without a StatusError/Unknown enum, the alternatives misreport non-existence harder than the status quo; Install will still surface the real error if Detect misclassifies. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wrangler-style onboarding for
dhq. Afterdhq hellosucceeds, detect AI coding agents on this machine and install the DeployHQ skill (already inskills/deployhq/) into each agent's native format. Also adds a standalonedhq skillscommand for non-interactive / re-install flows.Twelve agents supported:
~/.aider/deployhq-skill.md(+ wire-up note)<repo>/AGENTS.mdsentinel section~/.claude/skills/deployhq/tree.clinerules/deployhq.md~/.codex/AGENTS.mdsentinel section~/.continue/rules/deployhq.md~/.cursor/rules/deployhq.mdc~/.gemini/GEMINI.mdsentinel section.github/copilot-instructions.mdsentinel section.kiro/steering/deployhq.md$XDG_CONFIG_HOME/opencode/AGENTS.mdsentinel section~/.codeium/windsurf/memories/global_rules.mdsentinel sectionDesign highlights
Target.Scope()returnsScopeUserorScopeProject. Thedhq helloprompt only auto-offers ScopeUser targets — we never mutate a user's repo as a side effect of logging in. Project-scope targets are opt-in viadhq skills install --agent <name>.writeEmbeddedTree+ dotfile version marker.buildOwnedSkillFile+<!-- deployhq-skill v1 -->HTML comment for versioning.mergeSectionkeeps user content outside the sentinel markers byte-for-byte intact across (re)installs.install -> installproduces identical files; outdated installs upgrade silently.Noterinterface lets Aider surface the one wire-up step we explicitly refuse to automate (editing the user's~/.aider.conf.ymlis too risky for the scaffold).harness.Detectsays we're running inside (say) Claude Code, the skill installs without prompting — the user is using it right now, they want it.Test plan
go build ./...cleango vet ./...cleango test ./...— all packages green;internal/skillinstaller/has 60+ tests covering detect / install / idempotence / outdated / coexistence per targetgolangci-lint run— 0 issuesdhq skills listsmoke-tested locally — all 12 targets visible, statuses match the on-disk reality of the dev boxdhq helloafter the merge, verify the prompt fires and installs cleanly for at least Claude Code + Cursordhq skills install --agent copilotinside a test repo, verify.github/copilot-instructions.md+.github/copilot/deployhq/tree🤖 Generated with Claude Code
Summary by CodeRabbit
dhq skillscommand withlistandinstallto manage DeployHQ skill integrations across supported tools.dhq hellointeractive setup with a new step to offer local skill installation (errors won’t stop setup).dhq skills installinstalls detected user-scope skills, skips project-scope ones unless explicitly selected, and reports available/installed/outdated status while preserving existing user content.