docs(adr): add ADR 0092 always-on harness skills - #6378
Conversation
PR Summary by QodoDocument always-on harness skills via bootstrap context
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Site previewPreview: https://4ec51e92-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 12:05 PM UTC · Completed 12:20 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1. Unimplemented skill behavior presented as current
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| Fullsend bootstrap pastes that `SKILL.md` body onto the copied agent | ||
| definition for the run. The whole skill directory is still uploaded, so | ||
| `scripts/`, `references/`, and other companions stay on disk for the agent to | ||
| use by path. Keep always-on bodies short; put heavy procedure in companions. | ||
| See [ADR 0091](../../ADRs/0091-always-on-harness-skills.md). |
There was a problem hiding this comment.
1. Unimplemented skill behavior presented as current 📜 Skill insight ≡ Correctness
The guide and architecture documentation present apply: always as operational, claiming Bootstrap pastes always-on skill bodies into the agent definition, even though ADR 0091 says the implementation does not yet exist and the current parser/runtime silently treats these skills as on-demand. Until SkillMeta parsing and Bootstrap composition are implemented, the behavior must be documented as future functionality using a > **Planned:** callout with a tracking-issue link.
Agent Prompt
## Issue description
The user-facing guide and architecture documentation describe `apply: always` as operational, but ADR 0091 confirms that the implementation has not landed. The current runtime ignores the metadata and does not append the skill body to the agent definition, causing users to receive an on-demand skill rather than rules applied on every run.
## Issue Context
Current `SkillMeta` has no `apply` field, so YAML parsing discards it, and Claude Bootstrap uploads the original agent definition and skill directories without composing them. Rewrite the affected documentation as future behavior—or defer it until the parser and Bootstrap implementation lands—and use the required `> **Planned:**` blockquote with a link to the tracking issue.
## Fix Focus Areas
- docs/guides/user/customizing-with-skills.md[63-87]
- docs/architecture.md[166-170]
- docs/ADRs/0091-always-on-harness-skills.md[134-137]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| - agent-architecture | ||
| - codebase-context | ||
| - agent-infrastructure | ||
| - security-threat-model |
There was a problem hiding this comment.
2. relates_to lists four problems 📜 Skill insight ⚙ Maintainability
ADR 0091 references four problem documents in relates_to, exceeding the recommended maximum of three. Narrow the ADR's scope or split the decision if all four areas are essential.
Agent Prompt
## Issue description
The ADR's `relates_to` frontmatter contains four problem areas, exceeding the recommended maximum of three.
## Issue Context
Retain only directly governed problem areas, or split the decision if its scope genuinely spans all four.
## Fix Focus Areas
- docs/ADRs/0091-always-on-harness-skills.md[4-8]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| By default, a skill listed in the harness is **uploaded** and shown in the | ||
| runtime skill list. The model opens the full `SKILL.md` with the Skill tool | ||
| when it chooses to (`apply: on-demand`, or omit `apply`). |
There was a problem hiding this comment.
5. harness jargon lacks definition 📜 Skill insight ✧ Quality
The guide introduces the domain-specific term harness without defining it or linking to its glossary entry. Readers unfamiliar with Fullsend's harness model lack the context needed to apply the instructions.
Agent Prompt
## Issue description
The modified guide uses `harness` for the first time without a definition or glossary link.
## Issue Context
Link the first occurrence to the harness entry in `docs/glossary.md`, or define the term inline.
## Fix Focus Areas
- docs/guides/user/customizing-with-skills.md[63-67]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 1. **Default (no `apply`, or `apply: on-demand`)** — upload and list it. | ||
| The model opens it with the Skill tool when needed. Same as today. | ||
| 2. **`apply: always`** — after upload, bootstrap **pastes the `SKILL.md` body** | ||
| (markdown after the frontmatter) onto the **end of the copied agent |
There was a problem hiding this comment.
6. Runtime contract inlined in adr 📜 Skill insight ⚙ Maintainability
The ADR inlines a field-level runtime contract for apply, including defaults, copy targets, ordering, companion handling, and scanning semantics. This detailed contract should live under a versioned docs/normative/ path and be linked from the ADR.
Agent Prompt
## Issue description
ADR 0091 embeds the detailed compatibility and runtime contract for the new `apply` field.
## Issue Context
Create a versioned normative specification containing field values, defaults, copy behavior, ordering, and security-processing requirements; leave the ADR with the architectural decision and a link to that specification.
## Fix Focus Areas
- docs/ADRs/0091-always-on-harness-skills.md[74-117]
- docs/ADRs/0091-always-on-harness-skills.md[136-137]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Looks good to me Previous runLooks good to me Previous run (2)ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
ready-for-merge
|
|
||
| 1. **Default (no `apply`, or `apply: on-demand`)** — upload and list it. | ||
| The model opens it with the Skill tool when needed. Same as today. | ||
| 2. **`apply: always`** — after upload, bootstrap **pastes the `SKILL.md` body** |
There was a problem hiding this comment.
Pasting the SKILL.md body feels weird. There are other things in the skill. Sometimes references/. Sometimes scripts/. By pasting the content, the agent is exposed to the SKILL.md content without ever recording that it "used" the skill in claude codes own logs and traces.
What about adding a shorter line to the default prompt that just says:
ALWAYS use the following skills: `<user-provided-skill-a>`, `<user-provided-skill-b>`.
The user wants you to use those without fail.
There was a problem hiding this comment.
Agreed — we dropped body paste.
ADR 0091 is now the soft “ALWAYS use these skills …” path: names only in the prompt, open via the Skill tool.
Proof write-up (playground): https://github.com/fullsend-playground/python-app/blob/main/docs/adr-0091-skill-activation-matrix.html
|
|
||
| **Trade-offs:** Works today. Couples defaults to one team. Derived agent. | ||
|
|
||
| ### Option C: `apply: always` + load at bootstrap (chosen) |
There was a problem hiding this comment.
See https://agentskills.io/specification
Perhaps this should be metadata.apply, like this:
name: cool-skill
description: something
metadata:
apply: always
There was a problem hiding this comment.
Taken — Decision now uses metadata.apply: always (agentskills.io-style), with legacy top-level apply accepted for compatibility. Guide example updated to match.
|
I agree with Ralph, but I would go even further by injecting the phrase always and do not introduce a new key in the skill. Just And that is it. We can test how it goes. |
|
@ralphbean @rh-hemartin tossing this in now so you can react while I’m still testing — not waiting until I have a clean result. I’m leaning your way: don’t paste
I’m running this on playground, not changing the ADR Decision yet:
Early read (very not done): upload/list works. Soft “ALWAYS use this skill” was not enough on these triage runs — tools were Bash-only, so the model never actually opened the skill. Same “team add-on skills get skipped” thing we’ve seen with Caveman. If you hate the placeholder, the metadata key, or want a different experiment, yell now. I’ll keep iterating either way. |
029de12 to
b7097cf
Compare
What I tested in the playground orgMake triage use a Caveman skill every time: comment must start with
Always-on = Results
What “Listed only” vs “Always-on” meansBoth put Caveman on the harness so it gets uploaded. The difference is whether we force the model to open it. Listed only — skill on harness
Always-on — same Caveman rules, plus opt-in flag and name filled into the prompt.
Always-on worked
Listed only failed (no
|
|
🤖 Review · Commit: |
|
🤖 Finished Review · ✅ Success · Started 5:45 PM UTC · Completed 6:02 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $4.45 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
What about the placeholder minus the metadata key? |
|
@rh-hemartin on “placeholder minus the metadata key” — What that version would meanKeep the placeholder in the agent file (e.g. In your version there is no That is simpler. It is also a different product choice than what we proved with Caveman. What we proved (for context)Configs for those two tips:
So: listing alone does not open the skill. Naming it in the prompt (via the placeholder) does. The open question is only how we decide which names go into the placeholder. The case I want your take onShould we support: a team puts a skill on harness Example: triage
Which way do you lean — Having all skills listed under skills be always on or keep an opt-in (A or B)? |
Record Option B: metadata.apply always fills a soft Skill-tool directive with skill names (no SKILL.md body paste). Include playground activation matrix evidence and update architecture plus skills guide. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Keep ADR 0091 evidence out of fullsend; link the public playground HTML report instead of shipping the activation matrix in this PR. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
0091 on main is now per-agent runtime/model/effort; renumber this ADR and update cross-references accordingly. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ascerra
changed the title
c3ace2b to
94bd8d8
Compare
|
🤖 Finished Review · ✅ Success · Started 8:12 PM UTC · Completed 8:30 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.78 |
|
Risk Assessment: low (1/5) DetailsDocumentation-only PR adding ADR 0092 with cross-reference updates. Small change size, no protected paths, no security-sensitive files, docs-only with appropriate issue linkage. |
|
We are coming with this solution because we want to steer users to provide skills rather than modifying the system prompt. The problem here is that the agent is not using its skills when it should, because having it on the list does not make the agent explore and decide if a skill should be used. If you add a skill to an agent in Fullsend I think you care about consistency, as you took the effort to add it to the list. So I don't think the use-case you proposed makes sense in Fullsend. It makes sense locally and even then you would prompt the agent to use it instead of adding frontmatter to the skill. Another topic is skills that are not uploaded from the Fullsend side, but already exist on the target repo. Maybe folks want those used as well. I'm not sure if this PR is worth pursuing or we need to fallback to another approach, for example allowing users to add a "pre-prompt" so they can add "Agent please use these skills available on the repository" or just telling them directly to modify the system prompt. I will add this topic to today's meeting. |
|
Closing this issue as we have decided to go with a slightly different strategy outline here in this comment which led to 2 new issues being created and the closing of the original issue this PR set out to solve. #6380 (comment) |
|
🤖 Finished Retro · ✅ Success · Started 1:09 AM UTC · Completed 1:21 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.24 |
Retro: PR #6378 — ADR 0092 Always-On Harness SkillsHuman-authored docs PR proposing ADR 0092 (always-on harness skills). Over 9 days, human reviewers challenged the fundamental design approach, ultimately leading to the PR being closed without merge in favor of a different strategy. Timeline
Review Agent PerformanceThe review agent (from
Evidence for Existing Issues
Proposals filed
|
3 participants
Fixes #6380
Summary
metadata.apply: alwaysfills a soft always-use directive with skill names; the model opens each via the Skill tool. NoSKILL.mdbody paste.mainalready has ADR 0091 (per-agent runtime/model/effort).Related: #6352 (symptom), #237 (orthogonal inclusion policy).
Test plan
main; merge conflicts resolved