Skip to content

docs(skill): sync SKILL.md type enum with schema + add quickest-flow block#52

Merged
naorsabag merged 1 commit into
masterfrom
docs/skill-md-schema-sync
Apr 27, 2026
Merged

docs(skill): sync SKILL.md type enum with schema + add quickest-flow block#52
naorsabag merged 1 commit into
masterfrom
docs/skill-md-schema-sync

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented Apr 27, 2026

Summary

Real bug found during a live agent self-test (this session): SKILL.md taught two node types the schema rejects, and didn't document three it accepts. An agent following SKILL.md to the letter would write `type: transform` and hit a 400.

Changes

`skills/openhop/SKILL.md`

  • Sync the type enum with `packages/shared/src/schema.ts`:
    • Drop `transform`, `validation` (not in schema)
    • Add `docker`, `k8s`, `scheduler` (in schema, were undocumented)
  • Sync the Node Type Variants table with representative variants for the three new types
  • "Types are categories, labels are names" callout — `redis` is a label, not a type; `oauth` is a label, not a type
  • "Quickest valid flow" copy-paste block at the top — known-valid 3-tier skeleton agents can start from
  • Add `validate` and `get` to the CLI command table (they were missing)

`packages/shared/tests/skill-md-sync.test.ts` (new)

Locks in three invariants:

  1. Every type SKILL.md advertises is accepted by the schema
  2. Every schema type appears in the SKILL.md variants table
  3. The type-bullet line lists every schema type

If anyone adds a new node type without updating SKILL.md (or vice versa), CI fails. Drift is structurally impossible.

Test plan

  • 3/3 new sync tests pass
  • 79/79 in `@openhop/shared` (was 76)
  • `prettier --check` clean
  • `eslint` clean
  • CI green
  • CodeRabbit review addressed

Reproduction trail

Live this session, with `type: transform`-like guesses:
```
{"ok":false,"error":"validation","errors":[{"path":"flow.nodes.0.type",
"message":"Invalid option: expected one of "actor"|"endpoint"|"auth"
|"database"|"external"|"cache"|"queue"|"service"|"docker"|"k8s"
|"scheduler"|"custom""}]}
```

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added example flow configuration and validation rules
    • Updated CLI command reference with new operations and output options
    • Clarified node type schema and the distinction between type category and label
    • Enhanced guidance on supported node types and configuration constraints

…lock

Found during a live agent self-test: SKILL.md taught two type values
(transform, validation) the schema rejects, and didn't mention three
the schema accepts (docker, k8s, scheduler). An agent following SKILL.md
to the letter would write `type: transform`, hit a 400, and stall.

Changes to skills/openhop/SKILL.md:

- Sync the `type` enum (line ~181) to match packages/shared/src/schema.ts:
    drop:  transform, validation
    add:   docker, k8s, scheduler
- Sync the Node Type Variants table with the same set; add representative
  variants for the three new types (containers/sidecars; pods/deployments;
  cron/airflow/temporal/etc.).
- Add an explicit "types are categories, labels are names" callout —
  redis is a label, not a type; oauth is a label, not a type. Use
  `type: custom` when nothing fits.
- Add a "Quickest valid flow" copy-paste block at the top so agents
  start from a known-valid skeleton instead of improvising the schema.
  Includes the three most common gotchas: closed type enum, `data` is
  string-or-object (not array), id charset.
- Add `validate` and `get` to the CLI command table (they were missing).
  Note that every command supports --json and the semantic exit codes.

New test: packages/shared/__tests__/skill-md-sync.test.ts
- Locks in three invariants:
    1. every type SKILL.md advertises is accepted by the schema
    2. every schema type appears in the variants table
    3. the type-bullet line lists every schema type

If anyone adds a new node type to NodeTypeEnum without updating SKILL.md
(or vice versa), CI fails. Drift is now structurally impossible.

Tests: 79/79 in @openhop/shared (was 76).

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

coderabbitai Bot commented Apr 27, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

A new test suite is introduced to enforce bidirectional compatibility between documentation and schema definitions, alongside updates to documentation that clarify validation rules, CLI commands, and node type specifications.

Changes

Cohort / File(s) Summary
Test Suite
packages/shared/__tests__/skill-md-sync.test.ts
New test suite verifying bidirectional compatibility between SKILL.md and schema NodeTypeEnum.options. Derives known types from schema, parses the "Node Type Variants" table from documentation, and performs three validation checks: advertised types are schema-accepted, all schema types appear in documentation, and the type bullet list includes all NodeTypeEnum.options values.
Documentation
skills/openhop/SKILL.md
Documentation updates clarifying validation rules (closed type enum, data constraints, id character limits), CLI command reference (openhop validate, openhop get, openhop help --json, openhop serve), CLI usage patterns (universal --json output, semantic exit codes, pre-push validation), and schema node definitions (constrains label as freeform, type as closed enum with new values docker, k8s, scheduler, and default service). Replaces node-type variants table positioning examples as label values; adds "Critical" guidance separating category from name and directing unsupported categories to type: custom.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 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 and specifically identifies the main changes: synchronizing SKILL.md's type enum with the schema and adding a quickest-flow block, matching the PR's primary objectives.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/skill-md-schema-sync

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

@naorsabag naorsabag merged commit 335cc0e into master Apr 27, 2026
6 of 7 checks passed
@naorsabag naorsabag deleted the docs/skill-md-schema-sync branch May 8, 2026 06:30
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