review+fix: end-to-end audit found 9 gaps, fixed 3 in scope, filed rest for Claude [Pi]
This commit is contained in:
parent
f5a6a5bfcc
commit
d9e7fb71ca
3 changed files with 178 additions and 5 deletions
|
|
@ -149,7 +149,9 @@ Make executable: `chmod +x ~/.claude/hooks/dispatch-trigger.sh`
|
|||
|
||||
The existing dispatch skill is solid. These edits sharpen when to delegate vs solo, and what depth of dispatch to use.
|
||||
|
||||
### Threshold Rules (append to dispatch SKILL.md)
|
||||
### Threshold Rules (replace section in dispatch SKILL.md)
|
||||
|
||||
**Important: Do NOT append. Delete the existing `Auto-invoke` bullet entirely and replace it with the new text below.** The old section says "Auto-invoke (even without being typed)" which relies on the model's own judgment. The new section says "When you see the `[dispatch-check]` marker" which relies on an external hook. If both survive, the model gets mixed signals.
|
||||
|
||||
```
|
||||
## Thresholds — Delegate vs Solo vs Orchestrate
|
||||
|
|
@ -428,7 +430,7 @@ This skill exists because of the 2026-06-24 drift: Pi finished a workflow-consol
|
|||
|------|-----|------|------------|
|
||||
| 1 | Claude | Create dispatch-trigger.sh + chmod | Nothing |
|
||||
| 2 | Claude | Edit settings.json: add UserPromptSubmit hook block | Step 1 |
|
||||
| 3 | Claude | Edit dispatch SKILL.md: append Thresholds section, update When-to-Invoke | Step 2 |
|
||||
| 3 | Claude | Edit dispatch SKILL.md: **replace** old Auto-invoke section with new thresholded version, then append Thresholds section | Step 2 |
|
||||
| 4 | Claude | Create delegation-discipline SKILL.md | Step 3 |
|
||||
| 5 | Pi | Create dispatch-trigger.ts extension in ~/.pi/agent/extensions/ | Nothing |
|
||||
| 6 | Pi | Run /reload in Pi to activate the extension | Step 5 |
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ A second Stop hook entry in `settings.json` that auto-commits research directori
|
|||
|
||||
```
|
||||
signal/research/ — CGM/DMEPOS market research, compliance intelligence, competitive analysis
|
||||
signal/docs/ — Design documents, architecture specifications, whitepapers
|
||||
signal/docs/compliance/ — Compliance docs (data handling, privacy, incident response)
|
||||
signal/content/ — Blog drafts, outlines, published posts (created if doesn't exist)
|
||||
```
|
||||
|
|
@ -58,7 +59,7 @@ File: `~/.claude/hooks/research-autopush.sh`
|
|||
|
||||
REPO="/Users/sttil-solutions/projects/signal"
|
||||
EXCLUDE_PATTERNS="*.tmp scratch-* draft-*-wip.md *.json *.html"
|
||||
DIRS=("research" "docs/compliance" "content")
|
||||
DIRS=("research" "docs" "content")
|
||||
|
||||
cd "$REPO" 2>/dev/null || exit 0
|
||||
|
||||
|
|
@ -219,13 +220,13 @@ const RESEARCH_PATTERNS = [
|
|||
const EXCLUDE_PATTERNS = [/\.tmp$/, /scratch-/, /draft-.*-wip\.md$/, /\.json$/, /\.html$/];
|
||||
|
||||
export default function (pi: ExtensionAPI) {
|
||||
pi.on("tool_call", async (event, ctx) => {
|
||||
pi.on("tool_result", async (event, ctx) => {
|
||||
if (event.toolName !== "write" && event.toolName !== "edit") return;
|
||||
|
||||
const path = event.input.path as string | undefined;
|
||||
if (!path) return;
|
||||
|
||||
// Check if path matches a research directory
|
||||
// Check if path matches a research or docs directory
|
||||
const isResearch = RESEARCH_PATTERNS.some((p) => p.test(path));
|
||||
if (!isResearch) return;
|
||||
|
||||
|
|
|
|||
170
docs/review-audit-delegation-research-versioning-2026-06-24.md
Normal file
170
docs/review-audit-delegation-research-versioning-2026-06-24.md
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
# End-to-End Audit: Delegation Router + Research Versioning — Gap Analysis
|
||||
|
||||
**Date:** 2026-06-24
|
||||
**Auditor:** Pi
|
||||
**Scope:** Both design systems delivered this session
|
||||
|
||||
---
|
||||
|
||||
## Audit Method: Simulated End-to-End Trace
|
||||
|
||||
I traced 5 user flows across both systems, examining every step from prompt submission to output persistence. Verified against the actual files on disk (not just the design docs).
|
||||
|
||||
---
|
||||
|
||||
## Findings
|
||||
|
||||
### Finding 1 (HIGH) — UserPromptSubmit hook output injection is unverified
|
||||
|
||||
**System:** Delegation Router — Claude-side trigger
|
||||
**Flow:** Task-shaped prompt → dispatch-trigger.sh → [dispatch-check] marker
|
||||
**Gap:** The dispatch-trigger.sh script prints its output to stdout, expecting it to be injected into Claude's conversation context. But the actual behavior of Claude Code's `UserPromptSubmit` hook is **not documented in this setup**. If the hook's stdout goes to a log, stderr, or is silently discarded, the `[dispatch-check]` marker never reaches the model and the trigger never fires.
|
||||
|
||||
**Why it matters:** This is the entire trigger mechanism. If the hook doesn't inject stdout into context, the system has no trigger at all.
|
||||
|
||||
**Fix needed before building:** Claude must verify how `UserPromptSubmit` hooks work in Claude Code — specifically whether command stdout is injected into the conversation. Test with a simple echo hook before building the real one.
|
||||
|
||||
---
|
||||
|
||||
### Finding 2 (HIGH) — dispatch-trigger.sh $1 assumption unverified
|
||||
|
||||
**System:** Delegation Router — Claude-side trigger
|
||||
**Flow:** UserPromptSubmit hook → bash script → $1
|
||||
**Gap:** The script assumes the prompt is passed as the first command argument (`PROMPT="$1"`). If Claude Code's`UserPromptSubmit` hook passes the prompt via environment variable, stdin, or not at all, `$1` is empty and the script always exits without firing.
|
||||
|
||||
**Why it matters:** Same as Finding 1 — the trigger silently never works.
|
||||
|
||||
**Fix:** Verify hook argument API as part of Finding 1.
|
||||
|
||||
---
|
||||
|
||||
### Finding 3 (MEDIUM) — Pi extension hooks tool_call, not tool_result (auto-stage never fires)
|
||||
|
||||
**System:** Research Versioning — Layer 3 (Pi extension)
|
||||
**Flow:** Pi writes research file → extension auto-stages
|
||||
**Gap:** The `research-versioning.ts` extension hooks `tool_call`, which fires **before** the tool executes. The file doesn't exist yet, so `git add` fails silently (caught by try/catch). The auto-stage never actually works for Pi.
|
||||
|
||||
**Why it matters:** Layer 3 is completely non-functional for Pi mid-session writes. Only the Stop hook (Layer 1) catches Pi's research artifacts, and only at session end.
|
||||
|
||||
**Fix:** Change from `tool_call` to `tool_result` event, which fires after the file has been written.
|
||||
|
||||
---
|
||||
|
||||
### Finding 4 (MEDIUM) — Dispatch thresholds need merge, not append (conflicting guidance)
|
||||
|
||||
**System:** Delegation Router — dispatch skill edits
|
||||
**Flow:** Model reads dispatch skill → two trigger mechanisms
|
||||
**Gap:** The dispatch skill currently has an `Auto-invoke` section that says "Auto-invoke (even without being typed) when..." — which relies on the model's own judgment. The redesign replaces this with "When you see the `[dispatch-check]` marker in context..." — which relies on an external hook. The design says "append Thresholds section" but the old and new auto-invoke rules coexist and can conflict:
|
||||
|
||||
- Old rule: model decides when to auto-invoke (broad, implicit)
|
||||
- New rule: hook decides when to flag, model only acts on explicit marker (narrow, explicit)
|
||||
|
||||
If both sections survive, the model gets mixed signals. The old section must be fully replaced, not just supplemented.
|
||||
|
||||
**Why it matters:** Model confusion = inconsistent delegation behavior.
|
||||
|
||||
**Fix:** When Claude edits dispatch SKILL.md, the old `Auto-invoke` bullet must be completely deleted and replaced by the new `Auto-invoke (dispatch check fired by UserPromptSubmit hook)` section. Do NOT leave both.
|
||||
|
||||
---
|
||||
|
||||
### Finding 5 (LOW) — signal/docs/ is not covered by research auto-versioning
|
||||
|
||||
**System:** Research Versioning — all three layers
|
||||
**Flow:** Pi writes design doc to signal/docs/ → auto-versioning
|
||||
**Gap:** The research-versioning extension watches `research/`, `docs/compliance/`, and `content/` — but NOT `signal/docs/`. Design documents written by Pi or Claude to `signal/docs/` are not auto-staged by Layer 3, not auto-committed by Layer 1 (Stop hook), and only covered by the manual discipline of Layer 2.
|
||||
|
||||
The `design-review-persist` skill tells Pi to manually commit — but Claude has no equivalent prompt to do the same.
|
||||
|
||||
**Why it matters:** signal/docs/ is the canonical home for design documents. If neither agent auto-versions it, design docs drift.
|
||||
|
||||
**Fix:** Add `signal/docs/` to Layer 1 stop hook directory scope and Layer 3 extension watched paths. The exclusion patterns already prevent raw data and large files from being committed.
|
||||
|
||||
---
|
||||
|
||||
### Finding 6 (LOW) — Mid-session research commits stay local until next push cycle
|
||||
|
||||
**System:** Research Versioning — Layer 1 (Stop hook)
|
||||
**Flow:** Agent commits mid-session → Stop hook sees clean tree → exits without pushing
|
||||
**Gap:** When an agent follows the research-versioning discipline and commits mid-session, the working tree is clean. The Stop hook (research-autopush.sh) exits immediately because `git diff --quiet` shows no changes. Those commits are never pushed until the NEXT session triggers a research change that creates a new commit-and-push cycle.
|
||||
|
||||
**Why it matters:** If the machine crashes or the session is interrupted between sessions, mid-session commits exist only in the local repo and are lost.
|
||||
|
||||
**Fix (optional):** The Stop hook could run `git push` even when it doesn't commit. But this adds latency to every session close. Risk is low — acceptable for now.
|
||||
|
||||
---
|
||||
|
||||
### Finding 7 (LOW) — Chief-of-staff sequential dependency handling is untested
|
||||
|
||||
**System:** Delegation Router — dispatch thresholds
|
||||
**Flow:** Multi-domain sequential task → chief-of-staft orchestration
|
||||
**Gap:** The thresholds route "sequential handoffs between specialists" to chief-of-staff. But the actual capability of the skyremote `chief-of-staff-kit` plugin to handle dependencies (specialist B depends on specialist A's output) is unknown. The plugin may only support parallel dispatch, not sequential workflows.
|
||||
|
||||
**Why it matters:** If chief-of-staff can't sequence, multi-domain sequential tasks break.
|
||||
|
||||
**Fix:** Discovery task for Claude: test chief-of-staff with a sequential brief. If it handles ordering, document it. If not, add a note that sequential multi-domain tasks need manual chaining.
|
||||
|
||||
---
|
||||
|
||||
### Finding 8 (LOW) — Pi model behavior with injected messages is untested
|
||||
|
||||
**System:** Delegation Router — Pi-side trigger
|
||||
**Flow:** Pi extension injects [dispatch-check-pi] → model may or may not respond
|
||||
**Gap:** The dispatch-trigger.ts extension injects a customType message with `display: true`. Whether the model (deepseek-v4-flash) treats this as an actionable instruction or ignores it as session metadata is unknown without testing.
|
||||
|
||||
**Why it matters:** The Pi-side trigger may silently have zero effect.
|
||||
|
||||
**Fix:** Testing step: submit a task-shaped prompt to Pi and verify the model responds to the dispatch-check. If ignored, switch from `message` injection to `systemPrompt` modification.
|
||||
|
||||
---
|
||||
|
||||
### Finding 9 (LOW/KNOWN) — Skills directory is not git-versioned
|
||||
|
||||
**System:** Both — shared infrastructure
|
||||
**Flow:** Any skill file in ~/.claude/skills/
|
||||
**Gap:** All deployed skills (dispatch edits, delegation-discipline, design-review-persist, research-versioning) live in `~/.claude/skills/` which is NOT under version control. A machine failure loses all skill work.
|
||||
|
||||
**Status:** TRACKED. This is phase 2 "skills-as-git-repo" from the workflow-consolidation plan.
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
| # | Severity | Finding | System | Fix needed |
|
||||
|---|----------|---------|--------|------------|
|
||||
| 1 | **HIGH** | UserPromptSubmit hook output injection unverified | Delegation Router (Claude) | Verify hook behavior before building |
|
||||
| 2 | **HIGH** | dispatch-trigger.sh $1 assumption unverified | Delegation Router (Claude) | Verify hook argument API before building |
|
||||
| 3 | **MEDIUM** | Pi extension hooks tool_call not tool_result | Research Versioning (Pi) | Change event to `tool_result` |
|
||||
| 4 | **MEDIUM** | Dispatch thresholds need merge, not append | Delegation Router (dispatch) | Delete old Auto-invoke section, don't append |
|
||||
| 5 | **LOW** | signal/docs/ not covered by research auto-versioning | Research Versioning (all layers) | Add docs/ to watched directories |
|
||||
| 6 | **LOW** | Mid-session commits stay local until next push | Research Versioning (Stop hook) | Optional: push even on clean tree |
|
||||
| 7 | **LOW** | Chief-of-staff sequential handling untested | Delegation Router (thresholds) | Discovery test by Claude |
|
||||
| 8 | **LOW** | Pi model behavior with injected messages untested | Delegation Router (Pi trigger) | Testing step |
|
||||
| 9 | LOW/KNOWN | Skills not git-versioned | Both | Phase 2: skills-as-git-repo |
|
||||
|
||||
## Recommended Next Steps
|
||||
|
||||
**Critical (blockers to resolve before building):**
|
||||
|
||||
1. Claude: Verify UserPromptSubmit hook behavior — does stdout inject into context? Is `$1` the prompt?
|
||||
2. Pi: Fix research-versioning extension from `tool_call` to `tool_result`
|
||||
|
||||
**Design refinements (fix in design docs before handoff):**
|
||||
3. Clarify dispatch threshold merge instructions (replace old section, not append)
|
||||
4. Add `signal/docs/` to research versioning scope (all 3 layers)
|
||||
|
||||
**Discovery tasks for Claude:**
|
||||
5. Test chief-of-staff sequential dependency handling
|
||||
6. Document the hook argument API after verification
|
||||
|
||||
---
|
||||
|
||||
## Files Affected by Fixes
|
||||
|
||||
| Finding | File to Change | Change Type |
|
||||
|---------|---------------|-------------|
|
||||
| 3 | `~/.pi/agent/extensions/research-versioning.ts` | Edit: tool_call → tool_result |
|
||||
| 4 | `signal/docs/delegation-router-design-2026-06-24.md` | Edit: clarify merge vs append |
|
||||
| 5 | `signal/docs/research-versioning-design-2026-06-24.md` | Edit: add docs/ to scope |
|
||||
| 5 | `~/.pi/agent/extensions/research-versioning.ts` | Edit: add docs/ to RESEARCH_PATTERNS |
|
||||
| 5 | `signal/docs/research-versioning-design-2026-06-24.md` | Edit: add docs/ to Stop hook |
|
||||
| 1,2 | `signal/docs/delegation-router-design-2026-06-24.md` | Add: verification step before build
|
||||
Loading…
Reference in a new issue