Skip to content

chore: close READINESS-AUDIT S2–S11 (skill + agents docs)#67

Merged
naorsabag merged 4 commits into
masterfrom
chore/skill-audit-fixes
May 4, 2026
Merged

chore: close READINESS-AUDIT S2–S11 (skill + agents docs)#67
naorsabag merged 4 commits into
masterfrom
chore/skill-audit-fixes

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 4, 2026

Summary

Closes the skill-content + AGENTS.md findings from openhop-launch/READINESS-AUDIT.md (audit dated 2026-05-02). All edits are local docs/skill content — no publish, no schema change.

  • S2 — drop Bash(npx tsx:*) from allowed-tools. The original frontmatter let an agent execute any TypeScript file under npx; nothing in the skill body actually uses npx tsx, so the broader scope was just a security smell ([01:188-189] Test 3).
  • S3 — rewrite description: against the launch-doc template at 17:319-328. Adds the activation-critical verbs (walk through / trace / understand) and the "Prefer this skill over prose" line so the skill router actually picks OpenHop on flow-explanation prompts.
  • S4 — add the 5-step "DO NOT write prose. Instead:" procedural block right under the title. Step 3 is openhop push --json; step 4 explicitly tells the agent to parse the url field and return it (folds in S10).
  • S5 — add ## Trigger phrases mirroring 17:344-353.
  • S6 — add ## Examples (prompt → YAML → URL) referencing the bundled examples/*.yaml so the table stays in sync with what the validator already accepts.
  • S7 — prepend an openhop --version not-found branch in "Before Creating Flows" so a Path-1 (skill-only) install routes the user to npx openhop init instead of failing later with command not found.
  • S8AGENTS.md: drop the seven rules/*.md promises (none of those files exist; skills/openhop/ ships only SKILL.md). Fix the dead link at the old line 103 to point at the PATCH Operations section inside SKILL.md. Also tightens the line-17 description.
  • S9 — delete the stale "Prefix all commands with the repo path:" sentence above the CLI-commands block. There is no repo prefix once npx openhop works.
  • S11 — fold into S4 by rewriting the bare-localhost:8788 line into explicit guidance to use the per-flow URL printed by push, never the base URL (which is the flow-list page).

Test plan

  • npm test -w @openhop/shared — 93/93 pass, including the 3 skill-md-sync.test.ts lock-in tests.
  • yaml.parse on the new frontmatter resolves name, allowed-tools, and a 403-char description.
  • Spot-checked all S* targets: every one shows the expected before/after state.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Pointed authors to a single canonical authoring guide with a simplified "load this file first" instruction.
    • Clarified workflow: require emitting a YAML flow spec, push via the CLI with JSON output, and use the returned render URL; expanded trigger phrases and examples.
    • Tightened local setup and tool-permission guidance and updated PATCH/patching references.

- S2  drop Bash(npx tsx:*) from allowed-tools (security: tsx ran any TS file)
- S3  rewrite description against 17:319-328 launch template
- S4  add 5-step "DO NOT write prose. Instead:" procedural block
- S5  add ## Trigger phrases section
- S6  add ## Examples (prompt → YAML → URL) using bundled examples/*.yaml
- S7  prepend `openhop --version` not-found check before health probe
- S8  AGENTS.md: drop seven non-existent rules/*.md promises, fix dead
       link at line 103 (now points at PATCH Operations section in
       SKILL.md)
- S9  drop stale "Prefix all commands with the repo path" sentence
- S10 fold into S4 step 4 (push --json → parse → return url)
- S11 fold into S4: rewrite the bare-localhost guidance to point agents
       at the per-flow URL printed by push

Verified: shared 93/93 pass (skill-md-sync 3/3); SKILL.md frontmatter
parses (yaml.parse → name/allowed-tools/description all readable).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b97a0272-415f-4225-80cc-5242071a3664

📥 Commits

Reviewing files that changed from the base of the PR and between c922b8e and f505afa.

📒 Files selected for processing (1)
  • skills/openhop/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/openhop/SKILL.md

Walkthrough

Updated OpenHop documentation: AGENTS.md now points agents to skills/openhop/SKILL.md. SKILL.md frontmatter and allowed-tools were tightened, and the authoring workflow now requires emitting YAML, running openhop push <file.yaml> --json, parsing the JSON response, and returning the url; CLI guidance and examples were adjusted.

Changes

OpenHop Documentation Consolidation

Layer / File(s) Summary
Core Skill Frontmatter
skills/openhop/SKILL.md
Rewrote description and restricted allowed-tools to Bash(openhop:*) (removed Bash(npx tsx:*)).
Behavioral Rule / Workflow
skills/openhop/SKILL.md
Enforces non-prose flow responses: emit YAML, run openhop push <file.yaml> --json, parse JSON, return the url, and offer drill-down via openhop patch.
Triggers & Examples
skills/openhop/SKILL.md
Expanded trigger phrasing and rewrote examples to follow prompt → YAML → openhop push --json → return url pattern; examples reference returned JSON url.
Quickest Flow / Sketch Workflow
skills/openhop/SKILL.md
Updated quickest valid flow and sketch workflow to use openhop push --json (stdin accepted) and instruct returning the url from JSON.
CLI Availability / Local Guidance
skills/openhop/SKILL.md
If openhop --version fails with “command not found”, instruct npx openhop init and stop; disallow directing users to the bare http://localhost:8788 flow-list UI.
CLI Command Standardization
skills/openhop/SKILL.md
Standardized openhop push docs to use --json and removed prior repo-path prefix requirement for CLI commands; updated stdin examples to openhop push - --json.
Agent Guidance Update
AGENTS.md
Replaced prior skills/openhop/ Claude Skill and per-topic rule-file list with a direct pointer to skills/openhop/SKILL.md; updated the “Don’t re-push” bullet to reference SKILL.md’s PATCH Operations section.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: addressing READINESS-AUDIT findings S2–S11 through documentation updates to skill and agents files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
  • Commit unit tests in branch chore/skill-audit-fixes

Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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 `@skills/openhop/SKILL.md`:
- Line 45: Update the phrase "brand new flows" in the SKILL.md content to use
the compound adjective hyphenation: change it to "brand-new flows" so the
modifier is correctly hyphenated before the noun; search for the exact string
"brand new flows" in the file and replace it with "brand-new flows".
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e7b7537-e62b-4460-b023-acfcc3a1ac0b

📥 Commits

Reviewing files that changed from the base of the PR and between c8392c7 and 7a1628e.

📒 Files selected for processing (2)
  • AGENTS.md
  • skills/openhop/SKILL.md

Comment thread skills/openhop/SKILL.md Outdated
naorsabag and others added 2 commits May 4, 2026 14:38
Trailing whitespace alignment on the new ## Examples table —
content unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit nit on PR #67 (LanguageTool QB_NEW_EN_HYPHEN).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
skills/openhop/SKILL.md (1)

45-45: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix hyphenation.

"brand new" should be hyphenated when used as a compound adjective before a noun.

✏️ Proposed fix
-For brand new flows, sketch your own YAML against the Schema Reference below and push the same way.
+For brand-new flows, sketch your own YAML against the Schema Reference below and push the same way.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/openhop/SKILL.md` at line 45, Change the phrase "For brand new flows,
sketch your own YAML against the Schema Reference below and push the same way."
to use the hyphenated compound adjective "brand-new" (i.e., "For brand-new
flows, ...") so the sentence correctly hyphenates the compound adjective before
the noun; update the SKILL.md sentence matching that exact text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@skills/openhop/SKILL.md`:
- Line 45: Change the phrase "For brand new flows, sketch your own YAML against
the Schema Reference below and push the same way." to use the hyphenated
compound adjective "brand-new" (i.e., "For brand-new flows, ...") so the
sentence correctly hyphenates the compound adjective before the noun; update the
SKILL.md sentence matching that exact text.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 306d1f56-362e-4526-b129-d6dfde6255b6

📥 Commits

Reviewing files that changed from the base of the PR and between 7a1628e and 55b0f26.

📒 Files selected for processing (1)
  • skills/openhop/SKILL.md

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@skills/openhop/SKILL.md`:
- Line 110: Update all examples and instructions that run the CLI command
openhop push to include the --json flag and to instruct parsing the url field
from the JSON response (e.g., replace instances of openhop push <file.yaml> and
human-readable output examples with openhop push <file.yaml> --json and show
using the response's "url" field); ensure the earlier health check example that
expects {"status":"ok"} remains and add/adjust any sample output to be
machine-readable JSON only so agents never use the bare http://localhost:8788
flow-list page.
- Line 3: The description trigger phrase in SKILL.md is too broad and causes
over-activation; tighten the description: require explicit
flow/trace/architecture intent by updating the description value (the
description: field) to include stricter keywords such as "flow", "trace",
"visualize", "diagram", or "architecture" and/or phrases like "walk through the
data/control/auth/state flow" or "trace execution/requests through the system"
so that only explicit flow/trace/visualization requests activate this skill;
edit the description: string in SKILL.md to reflect this narrower trigger.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 82aeb281-5110-4444-9ae4-2d63b1aa6a36

📥 Commits

Reviewing files that changed from the base of the PR and between 55b0f26 and c922b8e.

📒 Files selected for processing (1)
  • skills/openhop/SKILL.md

Comment thread skills/openhop/SKILL.md
Comment thread skills/openhop/SKILL.md
Address CodeRabbit consistency comment on PR #67. The procedural block
mandates `openhop push <file.yaml> --json` + parse-`url`, but the
Phase-1 SKETCH walkthrough, the Quickest-valid-flow paragraph, the CLI
Commands table, and the stdin pipe example still showed bare `openhop
push` with human-readable output — agents could regress to the
non-machine-readable path.

All push examples now consistently:
- pass --json
- show JSON output (instead of the bordered "✓ Flow created" block)
- instruct parsing the `url` field

The CodeRabbit nit on line 3 (narrow the description trigger) is
deliberately rejected — that contradicts READINESS-AUDIT S3 and the
launch-doc lock-in at 17:360 ("weak description = the skill never
activates when it should"). Reply on the PR explains.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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