📝 Add adept discovery design pipeline artifacts and harness investigation goal
Design, plan, fact-check report, and athanor implementation report for the adept discovery and tiered knowledge folders feature. Also captures the harness behavior investigation as an open goal.
This commit is contained in:
@@ -0,0 +1,229 @@
|
||||
# Fact-check: Adept discovery and tiered knowledge folders
|
||||
|
||||
**Date**: 2026-09-18
|
||||
**Design**: `designs/2026-09-18-adept-discovery.md`
|
||||
**Plan**: `plans/2026-09-18-adept-discovery-plan.md`
|
||||
|
||||
---
|
||||
|
||||
## FC-1: Nested `CLAUDE.md` auto-loading
|
||||
|
||||
**Verdict: CONFIRMED**
|
||||
|
||||
**Evidence**: During this fact-check session, running `ls plugins/spire/agents/` caused the harness to inject `plugins/spire/CLAUDE.md` as a `<system-reminder>` block. This is direct empirical evidence that the harness auto-loads a subdirectory's `CLAUDE.md` when files beneath it are accessed (including via the Bash tool, not only the Read tool).
|
||||
|
||||
The behavior is consistent with Claude Code's documented CLAUDE.md / memory system, which processes `CLAUDE.md` files at each level of the directory tree. Four `CLAUDE.md` files exist in this repo: the root, `plugins/bevy/`, `plugins/g4b_ai/`, and `plugins/spire/`. The Spire one was injected on first access to files under `plugins/spire/`.
|
||||
|
||||
**Impact**: The design's core assumption holds. Keeping adept `CLAUDE.md` files short (under ~60 lines) is justified because they are auto-loaded whenever any file in the domain folder is read. The "index, not a document" principle is sound.
|
||||
|
||||
---
|
||||
|
||||
## FC-2: Skill shell injection with a script path
|
||||
|
||||
**Verdict: CONFIRMED (with caveats)**
|
||||
|
||||
### `${CLAUDE_SKILL_DIR}` resolution
|
||||
|
||||
**Confirmed.** The `spire:project` skill at `plugins/spire/skills/project/SKILL.md` already uses `${CLAUDE_SKILL_DIR}` successfully:
|
||||
|
||||
```
|
||||
!`cat "${CLAUDE_PROJECT_DIR}/.claude/spire/project.md" 2>/dev/null || cat "${CLAUDE_SKILL_DIR}/../igor/references/project.md"`
|
||||
```
|
||||
|
||||
The research doc (`docs/workpad/research/2026-09-17-project-md-override-mechanics.md`) explicitly documents `${CLAUDE_SKILL_DIR}` as a load-time substitution: "Skill subdirectory / plugin install directory (plugin skills only)."
|
||||
|
||||
### `allowed-tools` for `bash` vs `cat`
|
||||
|
||||
**Caveat identified.** Every existing skill in Spire uses `Bash(cat:*)` for its `allowed-tools`, not `Bash(bash:*)`:
|
||||
|
||||
- `spire:project` -- `allowed-tools: Bash(cat:*)`
|
||||
- `spire:init` -- `allowed-tools: Bash(cat:*), Bash(grep:*), Bash(ls:*), Bash(just:*), Bash(git:*), Read, Write, Glob, Grep`
|
||||
|
||||
The project's `settings.json` allow-list includes `Bash(cat:*)` but NOT `Bash(bash:*)`. The skill's `allowed-tools: Bash(bash:*)` should pre-approve the command for injection at load time (per the research doc: "declare `allowed-tools: Bash(cat:*)` so the command is pre-approved in any project"). However, `Bash(bash:*)` has never been tested in this project's plugin skills.
|
||||
|
||||
This is a novel pattern -- all existing injection uses `cat`, not `bash`. The design might alternatively use `Bash(sh:*)` or restructure the injection to source the script rather than invoking bash explicitly. Consider adding `Bash(bash:*)` to the project settings allow-list for defense-in-depth.
|
||||
|
||||
### `disableSkillShellExecution`
|
||||
|
||||
The design documents a fallback (Haiku subagent runs `roster.sh`). The research doc confirms: "`disableSkillShellExecution: true` in settings turns injection off."
|
||||
|
||||
**Impact**: The mechanism is sound in principle but uses an untested permission pattern (`Bash(bash:*)`). Recommend testing the exact `!`bash "${CLAUDE_SKILL_DIR}/roster.sh"`` injection in a scratch skill before committing, and adding `Bash(bash:*)` to the project `settings.json` allow-list.
|
||||
|
||||
---
|
||||
|
||||
## FC-3: Workpad resolution in the roster script
|
||||
|
||||
**Verdict: CONFIRMED (with documented gaps)**
|
||||
|
||||
The `spire:workpad` skill (`plugins/spire/skills/workpad/SKILL.md`) has a six-step resolution chain:
|
||||
|
||||
1. Explicit user instruction in conversation
|
||||
2. `WORKPAD_FOLDER` env var (via `just workpad-folder` or `printenv`)
|
||||
3. Project-local tickets folder (external)
|
||||
4. `docs/workpad` if `docs/` exists
|
||||
5. `.workpad`
|
||||
6. Ask the user
|
||||
|
||||
The roster script covers steps 2, 4, and 5:
|
||||
`WORKPAD_FOLDER` -> `just workpad-folder 2>/dev/null` -> `docs/workpad` -> `.workpad`
|
||||
|
||||
**Gaps**:
|
||||
|
||||
- **Step 1 (explicit user instruction)**: Impossible in a non-interactive script. Acceptable -- the script runs at skill load time, not during conversation.
|
||||
- **Step 3 (tickets folder)**: Not directly covered. The plan adds "grep `.claude/spire/project.md` for the `Workpad:` line" as a fallback. However, `.claude/spire/project.md` does NOT exist in this repo (init hasn't been run), and the file format is freeform markdown, not structured YAML -- `grep` extraction would be fragile.
|
||||
- **Step 6 (ask)**: Impossible in a non-interactive script. Acceptable -- the script's "none" fallback handles the missing-workpad case cleanly.
|
||||
- **Justfile `workpad-folder` recipe**: The Justfile in this repo exists but does NOT define a `workpad-folder` recipe (it only has `default` and `claude-rc`). The `2>/dev/null` suppression handles this correctly.
|
||||
|
||||
**Impact**: The gaps are acceptable for a derived-metadata script. The tickets-folder case is the only one that could produce a wrong result (finding no workpad when one exists externally). The plan's mitigation (grep `project.md`) is the right direction but needs a stable format to grep for (suggest a YAML frontmatter field like `workpad:` rather than freeform markdown).
|
||||
|
||||
---
|
||||
|
||||
## FC-4: `Skill("spire:adepts")` from a subagent
|
||||
|
||||
**Verdict: UNCERTAIN**
|
||||
|
||||
**Evidence**:
|
||||
|
||||
- The Skill tool is a core tool in Claude Code, available by default to all agents. No agent definition in the repo restricts it.
|
||||
- The adept agent (`plugins/spire/agents/adept.md`) lists `skills: [spire:workpad]` but NOT `spire:adepts`. This means `spire:adepts` would not be preloaded at boot but would need to be invoked on demand via model-driven activation.
|
||||
- Plugin skills are available to all sessions where the plugin is enabled (`"spire@reliquary": true` in `settings.json`). The `description` field in the skill's frontmatter drives model-based activation.
|
||||
|
||||
**Unknowns**:
|
||||
|
||||
- Whether a subagent spawned via the Agent tool has the same plugin skill access as the parent session has not been directly tested.
|
||||
- Whether shell injection in a skill (`!`bash ...``) works correctly when the skill is invoked by a subagent (rather than loaded at agent boot via `skills:` frontmatter) has not been tested.
|
||||
- The adept could alternatively add `spire:adepts` to its `skills:` list to guarantee preloading, but this injects the roster at boot for every adept, which may not be desired.
|
||||
|
||||
**Impact**: If subagent skill invocation doesn't work, the design falls back to the adept doing `ls` and `grep` on the adepts folder -- exactly the pattern the design aims to replace. The safest mitigation is to add `spire:adepts` to the adept agent's `skills:` list. The roster is one line per domain and cheap, so preloading at boot is acceptable. If on-demand invocation works, the `skills:` entry can be removed later.
|
||||
|
||||
---
|
||||
|
||||
## FC-5: `git log` with glob pathspecs
|
||||
|
||||
**Verdict: CONFIRMED**
|
||||
|
||||
**Evidence** (tested in this repo):
|
||||
|
||||
```bash
|
||||
# Quoted glob -- git interprets as pathspec
|
||||
git log --oneline HEAD~5..HEAD -- 'plugins/spire/skills/igor/*'
|
||||
# Output: bea38f0, 728db14, 4d081ae (3 commits)
|
||||
|
||||
# Unquoted glob -- shell expands, git gets explicit paths
|
||||
git log --oneline HEAD~5..HEAD -- plugins/spire/skills/igor/*
|
||||
# Output: identical (3 commits)
|
||||
```
|
||||
|
||||
Both produce identical results. The mechanism differs:
|
||||
|
||||
- **Quoted**: shell passes the literal `plugins/spire/skills/igor/*` to git, which interprets `*` as a pathspec glob (built into git).
|
||||
- **Unquoted**: bash expands the glob to matching files (`references SKILL.md`), then git gets explicit paths.
|
||||
|
||||
**Shell sensitivity**: In bash (which the Bash tool uses), a non-matching glob passes the literal string through. In zsh, a non-matching glob errors out by default. Since the drift check runs via the Bash tool (which uses bash), unquoted globs are safe. The user's login shell (fish) is irrelevant.
|
||||
|
||||
**Recommendation**: The handoff template should quote the paths in the drift check command for robustness:
|
||||
|
||||
```
|
||||
git log --oneline <sha>..HEAD -- 'src/auth/' 'src/middleware/auth*' 'tests/auth/'
|
||||
```
|
||||
|
||||
This ensures correct behavior regardless of which shell ends up running the command, and makes the intent explicit.
|
||||
|
||||
**Impact**: None -- the mechanism works. Quoting is a minor robustness improvement.
|
||||
|
||||
---
|
||||
|
||||
## FC-6: `sed` frontmatter parsing robustness
|
||||
|
||||
**Verdict: CONFIRMED (with noted edge cases)**
|
||||
|
||||
**Evidence** (tested with scratch files in `/tmp/`):
|
||||
|
||||
| Test case | `sed -n '2,/^---$/p'` output | Field extraction |
|
||||
|---|---|---|
|
||||
| Normal frontmatter | All fields + closing `---` | Correct |
|
||||
| Colon in value (`summary: Login flow: the auth chain`) | Correct | `Login flow: the auth chain` -- correct |
|
||||
| No frontmatter | Everything from line 2 to EOF | Empty -- no `^field:` matches |
|
||||
| Unclosed frontmatter (no closing `---`) | Everything from line 2 to EOF | Extracts fields correctly |
|
||||
| Empty summary (`summary:`) | Correct | Empty string -- correct |
|
||||
| Multiline YAML array (`covers:\n - src/auth/`) | Correct | `covers:` extraction returns empty (value on next line) |
|
||||
| Empty file | Empty | Empty |
|
||||
| Missing file (with `2>/dev/null`) | Empty, exit code 2 | Empty |
|
||||
|
||||
**Notable behaviors**:
|
||||
|
||||
1. The closing `---` IS included in the sed output (e.g., for normal frontmatter, the output is 5 lines: 4 fields + `---`). This does not affect field extraction because `---` does not match any `^fieldname: ` pattern.
|
||||
2. Files with no frontmatter cause sed to read the entire file through the pipeline. This is wasteful but harmless since (a) `CLAUDE.md` files are small, and (b) the field-extraction sed patterns won't match body content unless someone writes a line starting with `summary: ` in the prose body.
|
||||
3. The roster script only extracts `summary`, `status`, and `related` -- all single-line values. The plan correctly notes keeping `covers:` out of the roster (it's consumed by `handoff.md`'s drift check).
|
||||
|
||||
**Impact**: The parsing works for the design's purposes. The plan's risk note ("convention note that frontmatter values must be single-line") is appropriate. Consider adding a guard: if the first line is not `---`, skip the file (avoids reading entire non-frontmatter files through sed).
|
||||
|
||||
---
|
||||
|
||||
## FC-7: `ls` behavior for journal enumeration
|
||||
|
||||
**Verdict: CONFIRMED (with a minor concern)**
|
||||
|
||||
**Evidence** (tested with scratch directories in `/tmp/`):
|
||||
|
||||
| Test case | Behavior |
|
||||
|---|---|
|
||||
| Both `journal/` and domain root exist | Lists contents of both; grep filters correctly |
|
||||
| `journal/` doesn't exist | `2>/dev/null` swallows the error; domain root still listed |
|
||||
| Neither exists | Empty output, no errors |
|
||||
| `ls` with multiple directory args | Prints header labels (`/path/:`) per directory, but these don't match `^[0-9]{4}-` |
|
||||
|
||||
The `newest` extraction (`sort | tail -1 | cut -c1-13`) correctly produces the `YYYY-MM-DD-HH` timestamp from filenames. `cut -c1-13` maps exactly to the 13-character `YYYY-MM-DD-HH` format.
|
||||
|
||||
The `sessions` count (`grep -Eo | sort -u | wc -l`) correctly counts unique timestamps across both journal entries and legacy root-level files.
|
||||
|
||||
**Minor concern**: The `journal` directory entry itself appears in the `ls` output of the domain root, but it does not match the `^[0-9]{4}-[0-9]{2}-[0-9]{2}-[0-9]{2}` pattern, so it is correctly filtered out. Other non-journal files in the domain root (`CLAUDE.md`, `handoff.md`, `key-files.md`, etc.) are also filtered out by the same regex.
|
||||
|
||||
**Impact**: The mechanism works as designed. The backward-compatibility behavior (counting legacy root-level journal files alongside `journal/` entries) is intentional and correct for the lazy-migration story.
|
||||
|
||||
---
|
||||
|
||||
## FC-8: `CLAUDE_SKILL_DIR` scope
|
||||
|
||||
**Verdict: CONFIRMED**
|
||||
|
||||
**Evidence**:
|
||||
|
||||
The `spire:project` skill uses `${CLAUDE_SKILL_DIR}/../igor/references/project.md`. Given:
|
||||
|
||||
- `CLAUDE_SKILL_DIR` = `plugins/spire/skills/project/` (or its cached equivalent)
|
||||
- `../igor/references/project.md` navigates to `plugins/spire/skills/igor/references/project.md`
|
||||
|
||||
This file exists at `/home/g4b/Projects/reliquary/plugins/spire/skills/igor/references/project.md` -- verified. The path resolution is correct, confirming that `${CLAUDE_SKILL_DIR}` resolves to the **skill's own directory**, not the invoking context's directory.
|
||||
|
||||
For the proposed `spire:adepts` skill:
|
||||
- `CLAUDE_SKILL_DIR` would resolve to `plugins/spire/skills/adepts/` (or `~/.claude/plugins/cache/reliquary/spire/<version>/skills/adepts/` when cached)
|
||||
- `roster.sh` sits in the same directory as `SKILL.md`
|
||||
- `bash "${CLAUDE_SKILL_DIR}/roster.sh"` is a trivial same-directory reference -- no relative navigation needed
|
||||
|
||||
**Cache observation**: The current plugin cache (`~/.claude/plugins/cache/reliquary/spire/0.1.0/skills/`) is missing several recently added skills (`curator`, `init`, `project`). The source has 10 skills; the cache has 7. Any new skill (`adepts`) will also be missing until the cache is refreshed. This is an operational concern (re-install the plugin after adding the skill) but not a design flaw.
|
||||
|
||||
**Impact**: The mechanism is sound. `${CLAUDE_SKILL_DIR}/roster.sh` will resolve correctly.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| Item | Verdict | Design impact |
|
||||
|---|---|---|
|
||||
| FC-1: Nested CLAUDE.md auto-loading | **CONFIRMED** | None -- design assumption holds |
|
||||
| FC-2: Skill shell injection with script | **CONFIRMED** (caveat) | Test `Bash(bash:*)` permission; consider adding to settings.json allow-list |
|
||||
| FC-3: Workpad resolution in script | **CONFIRMED** (gaps) | Tickets-folder case needs structured format in `project.md`; acceptable for now |
|
||||
| FC-4: Skill invocation from subagent | **UNCERTAIN** | Add `spire:adepts` to adept agent's `skills:` list as safety net |
|
||||
| FC-5: git log with glob pathspecs | **CONFIRMED** | Quote paths in the drift check template for robustness |
|
||||
| FC-6: sed frontmatter parsing | **CONFIRMED** (edge cases) | Add first-line `---` guard; document single-line-value convention |
|
||||
| FC-7: ls for journal enumeration | **CONFIRMED** | None |
|
||||
| FC-8: CLAUDE_SKILL_DIR scope | **CONFIRMED** | None; refresh plugin cache after adding the skill |
|
||||
|
||||
**Overall assessment**: The design is well-grounded. Six of eight claims are confirmed outright or with minor caveats. One (FC-4, subagent skill invocation) is uncertain and has a simple mitigation (preload the skill). None are refuted. The plan can proceed with the following adjustments:
|
||||
|
||||
1. **FC-2**: Add `Bash(bash:*)` to the project `settings.json` allow-list or test the skill injection before committing.
|
||||
2. **FC-4**: Add `spire:adepts` to the adept agent's `skills:` frontmatter as a guaranteed-available fallback.
|
||||
3. **FC-5**: Quote glob paths in the `handoff.md` drift-check template.
|
||||
4. **FC-6**: Add a first-line guard (`head -1 | grep -q '^---$'`) to skip files without frontmatter early, avoiding full-file reads.
|
||||
5. **FC-8**: Remember to refresh the plugin cache after adding the new skill.
|
||||
Reference in New Issue
Block a user