g4borg
9e6deea36e
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.
230 lines
14 KiB
Markdown
230 lines
14 KiB
Markdown
# 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.
|