-
Notifications
You must be signed in to change notification settings - Fork 1
feat(rules): add code comment placement rule #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ SKILLS_FILE := $(dir $(lastword $(MAKEFILE_LIST)))SKILLS.txt | |
| SKILLS_LOCK_FILE := $(dir $(lastword $(MAKEFILE_LIST)))skills-lock.json | ||
| SKILLS_EXTERNAL_SOURCE_DIR := $(HOME)/.agents/skills | ||
| SKILLS_GLOBAL_LOCK := $(HOME)/.agents/.skill-lock.json | ||
| SKILLS_PROJECT_DIR := $(abspath $(dir $(lastword $(MAKEFILE_LIST)))) | ||
| SKILLS_CLI := $(SKILLS_PROJECT_DIR)/node_modules/.bin/skills | ||
|
|
||
| MCP_SRC := $(dir $(lastword $(MAKEFILE_LIST))).ruler/mcp.json | ||
| MCP_TARGET_DIRS := $(HOME)/.cursor $(HOME)/.claude $(HOME)/.codex | ||
|
|
@@ -87,6 +89,10 @@ skills-install: ## Install external skills from skills-lock.json (skips already | |
| @lock="$(SKILLS_LOCK_FILE)"; \ | ||
| skills_dir="$(SKILLS_EXTERNAL_SOURCE_DIR)"; \ | ||
| force="$${DOTAGENTS_FORCE_SKILLS_INSTALL:-0}"; \ | ||
| if ! (cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null); then \ | ||
| echo "Error: failed to install the skills SDK from bun.lock"; \ | ||
| exit 1; \ | ||
| fi; \ | ||
| if [ ! -f "$$lock" ]; then \ | ||
| echo "Error: $$lock not found"; \ | ||
| exit 1; \ | ||
|
|
@@ -109,7 +115,7 @@ skills-install: ## Install external skills from skills-lock.json (skips already | |
| skill_args=$$(printf '%s\n' "$$names" | while IFS= read -r n; do printf ' --skill %s' "$$n"; done); \ | ||
| count=$$(printf '%s\n' "$$names" | wc -l | tr -d ' '); \ | ||
| echo "Installing $$count skill(s) from $$source..."; \ | ||
| bun x skills add "$$source" --global --yes $$skill_args </dev/null; \ | ||
| $(SKILLS_CLI) add "$$source" --global --yes $$skill_args </dev/null; \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Quote
Proposed fix- $(SKILLS_CLI) add "$$source" --global --yes $$skill_args </dev/null; \
+ "$(SKILLS_CLI)" add "$$source" --global --yes $$skill_args </dev/null; \
- @$(SKILLS_CLI) update --global --yes </dev/null
+ @"$(SKILLS_CLI)" update --global --yes </dev/null
- echo "warn: no installed skills for install-all repo $$repo; run: bun install --frozen-lockfile --minimum-release-age 0 && $(SKILLS_CLI) add $$repo --global --yes --skill '*'"; \
+ echo "warn: no installed skills for install-all repo $$repo; run: bun install --frozen-lockfile --minimum-release-age 0 && \"$(SKILLS_CLI)\" add $$repo --global --yes --skill '*'"; \Also applies to: 141-141, 161-161 🤖 Prompt for AI Agents |
||
| status=$$?; \ | ||
| still_missing=$$(printf '%s\n' "$$names" | while IFS= read -r n; do \ | ||
| if [ ! -e "$$skills_dir/$$n" ] && [ ! -L "$$skills_dir/$$n" ]; then printf ' %s' "$$n"; fi; \ | ||
|
|
@@ -131,15 +137,16 @@ skills-refresh: ## Force a reinstall of all external skills from skills-lock.jso | |
|
|
||
| .PHONY: skills-update | ||
| skills-update: ## Update installed external skills to latest and refresh the lock. | ||
| @bun x skills update --global --yes </dev/null | ||
| @cd "$(SKILLS_PROJECT_DIR)" && bun install --frozen-lockfile --minimum-release-age 0 --no-progress >/dev/null | ||
| @$(SKILLS_CLI) update --global --yes </dev/null | ||
| @$(MAKE) skills-lock | ||
|
|
||
| .PHONY: skills-lock | ||
| skills-lock: ## Regenerate skills-lock.json from SKILLS.txt. | ||
| @global_lock="$(SKILLS_GLOBAL_LOCK)"; \ | ||
| skills_dir="$(SKILLS_EXTERNAL_SOURCE_DIR)"; \ | ||
| if [ ! -f "$$global_lock" ]; then \ | ||
| echo "Error: $$global_lock not found; install a skill first (bun x skills add ... --global) to initialize it."; \ | ||
| echo "Error: $$global_lock not found; install a skill first ($(SKILLS_CLI) add ... --global) to initialize it."; \ | ||
| exit 1; \ | ||
| fi; \ | ||
| if ! jq -e '(.version | type == "number") and (.skills | type == "object")' "$$global_lock" >/dev/null; then \ | ||
|
|
@@ -151,7 +158,7 @@ skills-lock: ## Regenerate skills-lock.json from SKILLS.txt. | |
| jq --argjson ondisk "$$ondisk" --argjson spec "$$spec" '. as $$lock | ($$lock.skills | with_entries(select(.key as $$k | $$ondisk | index($$k))) | with_entries(.value |= ({source, sourceType, sourceUrl, ref, skillPath, skillFolderHash} | with_entries(select(.value != null))))) as $$inst | reduce $$spec[] as $$s ({}; if ($$s.names | length) == 0 then . + ($$inst | with_entries(select(.value.source | ascii_downcase == ($$s.repo | ascii_downcase)))) else reduce $$s.names[] as $$n (.; ($$inst[$$n] // null) as $$hit | .[$$n] = (if $$hit != null and (($$hit.source | ascii_downcase) == ($$s.repo | ascii_downcase)) then $$hit elif .[$$n] != null then .[$$n] elif ($$s.repo | test("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$$")) then {source: $$s.repo, sourceType: "github", sourceUrl: "https://github.com/\($$s.repo).git"} else {source: $$s.repo} end)) end) | {version: $$lock.version, skills: (to_entries | sort_by(.key) | from_entries)}' "$$global_lock" > "$(SKILLS_LOCK_FILE).tmp" && mv "$(SKILLS_LOCK_FILE).tmp" "$(SKILLS_LOCK_FILE)"; \ | ||
| for repo in $$(printf '%s' "$$spec" | jq -r '.[] | select(.names | length == 0) | .repo'); do \ | ||
| if ! jq -e --arg repo "$$repo" '[.skills[] | select(.source | ascii_downcase == ($$repo | ascii_downcase))] | length > 0' "$(SKILLS_LOCK_FILE)" >/dev/null; then \ | ||
| echo "warn: no installed skills for install-all repo $$repo; run: bun x skills add $$repo --global --yes --skill '*'"; \ | ||
| echo "warn: no installed skills for install-all repo $$repo; run: bun install --frozen-lockfile --minimum-release-age 0 && $(SKILLS_CLI) add $$repo --global --yes --skill '*'"; \ | ||
| fi; \ | ||
| done; \ | ||
| undeclared=$$(jq -r --argjson ondisk "$$ondisk" --slurpfile out "$(SKILLS_LOCK_FILE)" '.skills | keys[] | . as $$k | select(($$ondisk | index($$k)) and ($$out[0].skills | has($$k) | not))' "$$global_lock" | paste -sd, -); \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Dependency upgrade log | ||
|
|
||
| ## 2026-08-06 | ||
|
|
||
| - `skills`: updated from `^1.5.20` to `^1.5.22`, the current stable release. | ||
| - `bun.lock` records the resolved `skills@1.5.22` package and its new transitive dependencies. | ||
| - The Makefile now invokes the project-local SDK binary and refreshes dependencies with Bun's release-age check disabled, so the declared current release is available immediately. | ||
| - Verification: `./node_modules/.bin/skills --version`, frozen-lockfile install, `make skills-install` from both `dotagents/` and the upstream `~/dotfiles` checkout, and `make sync`. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |
| "@biomejs/biome": "2.3.11", | ||
| "@intellectronica/ruler": "^0.3.23", | ||
| "lefthook": "^2.0.15", | ||
| "skills": "^1.5.20" | ||
| "skills": "^1.5.22" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
if [ -e node_modules/.bin/skills ]; then
readlink node_modules/.bin/skills || true
head -n 1 node_modules/.bin/skills
fi
command -v node >/dev/null && node --version || echo "node is not on PATH"
command -v bun >/dev/null && bun --version || echo "bun is not on PATH"
fd -H -t f '^(package\.json|\.nvmrc|\.node-version|\.tool-versions|Dockerfile.*)$' . \
-x sh -c 'printf "\n--- %s ---\n" "$1"; rg -n "engines|node|bun|setup-node|setup-bun" "$1" || true' sh {}Repository: shunkakinoki/dotagents Length of output: 523 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '--- package.json ---\n'
cat -n package.json
printf '\n--- lockfile skills package entry ---\n'
if [ -f bun.lock ]; then
python3 - <<'PY'
from pathlib import Path
p = Path('bun.lock')
text = p.read_text()
for i,line in enumerate(text.splitlines(), 1):
if '"skills' in line or 'skills@' in line or 'name": "skills"' in line:
start=max(1,i-8); end=min(len(text.splitlines()), i+15)
for n in range(start,end+1):
print(f"{n:4d}: {text.splitlines()[n-1]}")
print("...")
PY
fi
printf '\n--- files mentioning runtime/tooling ---\n'
fd -H -t f '^(package\.json|bun\.lock$|\.nvmrc|\.node-version|\.tool-versions|Dockerfile.*|Makefile|\.github/workflows/.*)$' . \
-x sh -c 'printf "\n--- %s ---\n" "$1"; rg -n "engines|node|bun|setup-node|setup-bun|node_modules/.bin/skills|skills:" "$1" || true' sh {}Repository: shunkakinoki/dotagents Length of output: 251 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- package.json ---'
cat -n package.json
printf '%s\n' ''
printf '%s\n' '--- lockfile skills package entry ---'
if [ -f bun.lock ]; then
python3 - <<'PY'
from pathlib import Path
p = Path('bun.lock')
text = p.read_text()
for i,line in enumerate(text.splitlines(), 1):
if '"skills' in line or 'skills@' in line or 'name": "skills"' in line:
start=max(1,i-8); end=min(len(text.splitlines()), i+15)
for n in range(start,end+1):
print(f"{n:4d}: {text.splitlines()[n-1]}")
print("...")
PY
fi
printf '%s\n' ''
printf '%s\n' '--- files mentioning runtime/tooling ---'
find . \( -type f \( -name package.json -o -name bun.lock -o -name .nvmrc -o -name .node-version -o -name .tool-versions -o 'iname Dockerfile*' \) -o -path '.github/workflows/*' \) -print0 2>/dev/null | sort -z | xargs -0 -r sh -c 'printf "%s\n" "--- $1 ---"; rg -n "engines|node|bun|setup-node|setup-bun|node_modules/.bin/skills|skills:" "$1" || true' sh {}Repository: shunkakinoki/dotagents Length of output: 5786 🌐 Web query:
💡 Result: The vercel-labs/skills package (npm: skills), specifically version 1.5.22, is the CLI tool for the open agent skills ecosystem [1][2][3]. Key details from its package.json and configuration: Node.js Engine Requirements: The package.json for version 1.5.22 declares the following engine requirement [1]: "engines": { "node": ">=22.20.0" } Note that while older documentation and previous versions of the package historically claimed support for Node.js 18+ [4][5][6], the repository has updated its requirements to align with its build and runtime behavior, which relies on features available in newer Node.js versions (specifically those supporting native TypeScript execution or newer ESM exports) [4][5]. Binaries (CLI): The package defines its executable entry points in the bin field of package.json [1]: "bin": { "skills": "./bin/cli.mjs", "add-skill": "./bin/cli.mjs" } Both the primary command, skills, and its alias, add-skill, map to the same file, bin/cli.mjs [1][7][8]. This file acts as a wrapper that manages the execution flow for the CLI [7][8]. General Information: - Package Manager: The project uses pnpm, as specified in package.json by "packageManager": "pnpm@10.17.1" [1][6]. - Module System: The package is configured as an ESM-only package ("type": "module") [1]. - Functionality: The CLI is used to install, manage, and list skills for various AI agents (e.g., Claude Code, OpenCode, Cursor) [2][8][3]. It can be invoked directly via npx skills without local installation [6][7][8]. Citations:
Document or enforce Node 22.20.0+ for
🤖 Prompt for AI AgentsSource: MCP tools |
||
| }, | ||
| "packageManager": "bun@1.2.22" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Code Comments | ||
|
|
||
| - Write only non-obvious WHY: hidden constraints, why a workaround exists, surprising behavior. | ||
| - No WHAT comments. `// get the user ID` is zero information. | ||
| - No change history. That belongs in `git log` and the PR. | ||
| - No task ID references. Put the needed context in the comment itself. | ||
| - Never pad uncertain code with comments. Flag uncertainty in the PR body, not the source. | ||
| - Docs and README: current behavior only, no rationale trails or migration history. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: shunkakinoki/dotagents
Length of output: 160
🌐 Web query:
Bun publish minimumReleaseAge lockfile minimum-release-age bunfig.toml💡 Result:
The
minimumReleaseAgesetting is a feature forbun installdesigned to prevent the installation of recently published packages (to protect against supply chain attacks) [1][2]. It is not a feature related tobun publish[3][4]. Forbun install,minimumReleaseAgecan be configured in yourbunfig.tomlfile under the[install]section [1][2]. Configuration in bunfig.toml: [install] # Accepts a number of seconds (e.g., 259200 for 3 days) or an ms-style duration string (e.g., "3d", "1 week") minimumReleaseAge = "3d" # Optional: List packages to bypass this check minimumReleaseAgeExcludes = ["@types/bun", "typescript"] Key details regardingminimumReleaseAge: - Scope: This setting affects dependency resolution duringbun installandbun add, not the publishing process [1][5]. - Input format: It supports both a raw number of seconds or a duration string (e.g., "3d", "48h", "1 week") [6]. - Exclusion: You can useminimumReleaseAgeExcludesto exempt specific packages from this age gate [2]. -bun publish: This command does not useminimumReleaseAge[3][4]. When runningbun publish, you can control lockfile behavior using flags like--no-lockfile(to not update the lockfile) or--disallow-lockfile-changes[3][4]. If you are seeing issues with this setting, note that it should be defined in a project-localbunfig.toml[7]. There have been reports of global$HOME/.bunfig.tomlconfigurations being ignored in certain environments [7].Citations:
bun installoven-sh/bun#22679[install] minimumReleaseAgein$HOME/.bunfig.tomlsilently ignored (project-local works) oven-sh/bun#30750🌐 Web query:
Bun install minimumReleaseAge frozen lockfile does not fail💡 Result:
In earlier versions of Bun, the
minimumReleaseAgeconfiguration was only enforced during dependency resolution (e.g., when adding new packages or updating dependencies) [1]. Historically, this meant that if a package version was already pinned in thebun.lockfile,bun installandbun install --frozen-lockfilewould silently install that version, even if it violated theminimumReleaseAgecooldown, essentially bypassing the protection [1]. However, this behavior has been addressed to ensure better security [1]. A fix was implemented (referenced in Bun issue#30525) wherebun install --frozen-lockfilenow performs a pre-install gate check [1]. This check walks thebun.lockfile, verifies the publication timestamps of the pinned versions against theminimumReleaseAgethreshold, and will fail (exit with an error) if any pinned version violates the cooldown [1]. If you find that yourminimumReleaseAgeis being ignored, ensure you are running a version of Bun that includes this fix [1]. Additionally, note thatminimumReleaseAgeis intended to gate resolution; if you are encountering issues where it appears ignored, check that you are not accidentally bypassing it viaminimumReleaseAgeExcludesor by setting the age to0[1][2][3]. If a lockfile was generated before a strictminimumReleaseAgewas enforced,bun install --frozen-lockfileshould now correctly identify and error on those previously-allowed, now-blocked versions [1].Citations:
Remove the release-age override if supply-chain protection is desired.
Lines 92 and 140 pass
--minimum-release-age 0, which disables Bun’sminimumReleaseAgepre-install gate. If this is not intentional, remove the flag from bothbun install --frozen-lockfilecommands so pinned lockfile entries are still age-checked.🤖 Prompt for AI Agents
Source: MCP tools