feat(spec): declared ### Skills section with fail-closed compile resolution#62
Conversation
| Rules: | ||
|
|
||
| - `### Skills` is the only place to declare harness-skill dependencies. The | ||
| declaration is not duplicated in frontmatter — frontmatter stays structural |
There was a problem hiding this comment.
we can remove lines like this.
Common claude/codex failure pattern: I often see design decisions creep into implementation. They proposed adding it to the front-matter, we said no, and then they propagate a description of that decision into the actual implementation when it would be better to just not say it at all.
I see this all the time.
| 3. `~/.codex/skills/`. | ||
| 4. `~/.agents/skills/`. | ||
| - If a declared skill cannot be resolved in any of those paths, both `prose | ||
| compile` and `prose run` fail closed with a `skill_unresolved` diagnostic |
There was a problem hiding this comment.
Here it says prose run fails, but in the PR description it says we deferred that decision and that prose run will not fail.
There was a problem hiding this comment.
I think it's fine to do it just in the prose compile step, but I'm curious why it came to the decision to defer it for prose run to make sure I'm not missing anything.
| @@ -0,0 +1,226 @@ | |||
| import { readdir, readFile, stat } from "node:fs/promises"; | |||
There was a problem hiding this comment.
I don't think we want the logic for "searching for skills that are required" to be a harness responsibility.
It should be a compiler/program-level responsibility.
Keep in mind the goal for prose is to still be harness-agnostic, and if we put this logic here instead of in the compiler (in markdown), then running a Prose-with-Skills program on a different harness will not behave as we expect.
…lution Implements the spec from issue #60. Components declare required harness skills via a `### Skills` section (colon form, e.g. `document-skills:pdf`). `prose compile` resolves declared skills against ./skills/, ~/.claude/skills/, ~/.codex/skills/, and ~/.agents/skills/, and fails closed with `skill_unresolved` before forwarding to the agent harness when any are missing. - Spec: skills/open-prose/contract-markdown.md gains a ### Skills row in the Canonical Sections table and a ## Skills H2 covering colon naming, search order, the BYO-harness invariant, and fail-closed semantics. - Implementation: tools/cli/src/skills/declared.ts (parser + resolver + directory walker + DeclaredSkillsUnresolvedError). Pure functions; no I/O beyond readFile / readdir / stat. - Wiring: tools/cli/src/commands/compile.ts pre-checks declared skills before forwarding the compile prompt; fails closed with CompileValidationError when any are unresolved. Gated behind the existing skillPreflight option for test parity. - Example: skills/open-prose/examples/declared-skills/ shows the document-skills:pdf canonical pattern. - Tests: 19 new (18 in declared.test.ts covering parser/resolver/walker/ error formatter; 1 in cli.test.ts asserting compile fails closed before the harness is invoked when a declared skill is missing). - BYO harness: OpenProse never installs harness skills; resolution failure is the user's signal to install the named skill themselves. Resolves #60. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…r review Address review feedback on #62: - contract-markdown.md: drop the "not duplicated in frontmatter" clause — the rejected alternative shouldn't propagate into the spec. - contract-markdown.md: fail-closed clause now mentions only `prose compile`; `prose run` enforcement is deferred per the PR description. - compiler/index.prose.md: add a `skills_resolver` agent that owns the search-path order, scope aggregation, BYO invariant, and fail-closed semantics. Skill resolution is now a compiler/program-level responsibility, not a harness responsibility, so other harnesses running the compiler get the same behavior. - skills/declared.ts: add a header comment pointing at the program-level spec; this module is the harness implementation of `skills_resolver`. - examples/declared-skills/README.md: update wording to reference the compiler agent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
eac2424 to
d4ff12a
Compare
|
Dan, yes, the review comments on #62 were addressed in
|
Implements the spec from #60 with the maintainer's review applied verbatim.
Maintainer's decisions from #60 (applied)
### Skillscanonical sectionskills/open-prose/contract-markdown.md(Canonical Sections + Skills H2)skills:frontmatter form### Skills(not### Harness Skills)./skills,~/.claude/skills,~/.codex/skills,~/.agents/skillstools/cli/src/skills/declared.ts(DECLARED_SKILL_SEARCH_DIRECTORIES)~/.agents/skillsaddedprose compiletools/cli/src/commands/compile.ts(preflightDeclaredSkillsForCompile)What changed (8 files, +823)
skills/open-prose/contract-markdown.mdgains a### Skillsrow in the Canonical Sections table and a short## SkillsH2 covering colon naming, search order, the BYO invariant, and fail-closed semantics.tools/cli/src/skills/declared.ts(new). Pure TS:parseDeclaredSkills,resolveDeclaredSkill,resolveDeclaredSkillsForFile,preflightDeclaredSkillsInRoot,DeclaredSkillsUnresolvedError. No I/O beyondreadFile/readdir/stat.tools/cli/src/commands/compile.tspre-checks declared skills before forwarding the compile prompt; throwsCompileValidationErrorwith askill_unresolvedheading and per-skill detail lines listing the searched paths. Pre-forward, so unresolved skills do not burn agent tokens. Gated behind the existingskillPreflightoption for test parity.skills/open-prose/examples/declared-skills/shows thedocument-skills:pdfcanonical pattern from the issue.tools/cli/tests/skills/declared.test.tscovering parser, resolver, directory walker, error formatter, and edge cases (case-insensitive header, dedupe, backticked names, hidden-dir skipping).tools/cli/tests/cli/cli.test.tsassertingprose compilerejects withCompileValidationErrorbefore the harness is invoked when a declared skill is missing.[Unreleased]entry references Spec proposal: declared agent skills on components #60.Total test count: 158 → 177 (all green, typecheck clean).
Test plan
npx vitest run— 177 / 177 passnpx tsc -p tsconfig.json --noEmit— clean.prose.mddeclaringdocument-skills:pdfagainst an empty HOME triggersskill_unresolvedwith searched paths, harness uninvokedNotes
Branched off
origin/mainthis time (last attempt was anchored to the abandonedrfc/reactive-openprosebranch — that PR (#59) is closed). No frontmatter form, per maintainer feedback that frontmatter stays structural.prose runenforcement was deliberately deferred — onceprose compileis fail-closed, a successful compile guarantees declared skills resolved at compile time, and runtime enforcement can be a follow-up if needed.Closes #60.
Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com