feat: quest autopsy — failure memory across sessions#75
Conversation
#67) When quests fail, lessons now persist as structured autopsy records in .fellowship/autopsies/. Future quests scan autopsies during Research and incorporate relevant warnings before repeating the same mistakes. CLI commands: - fellowship autopsy create: write autopsy from JSON stdin - fellowship autopsy scan: find autopsies by files, modules, or tags - fellowship autopsy infer: reconstruct autopsy from worktree signals Also simplifies gate guard to allow all fellowship CLI commands through the gate (not just gate approve/reject/init), since fellowship commands operate on state files, not source code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new autopsy subsystem with CLI commands (create, scan, infer), on-disk JSON persistence under .fellowship/autopsies, expiry/pruning, inference from worktree/tome/herald data, datadir expiry config, guard allowlist updates, tests, and docs/skill integration. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "fellowship CLI"
participant Autopsy as "internal/autopsy"
participant Datadir as "datadir / .fellowship"
participant Worktree as "worktree (tome/herald/eagles)"
CLI->>Autopsy: autopsy infer --dir <worktree> --repo <repo>
Autopsy->>Worktree: read quest-tome.json, herald, eagles (infer helpers)
Autopsy-->>Autopsy: build CreateInput (quest, trigger, files, modules, what_failed)
Autopsy->>Datadir: write .fellowship/autopsies/<ts>-<sanitized>.json
Datadir-->>Autopsy: persist confirmation
Autopsy-->>CLI: return created file path / success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
cli/internal/autopsy/autopsy_test.go (2)
112-126: Consider checking errors in test assertions.Lines 113-115 ignore the error from
os.ReadFileandjson.Unmarshal. While this is unlikely to cause issues in a controlled test, silently ignoring errors can mask test failures.🔧 Suggested fix
- data, _ := os.ReadFile(path) - var a Autopsy - json.Unmarshal(data, &a) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading autopsy file: %v", err) + } + var a Autopsy + if err := json.Unmarshal(data, &a); err != nil { + t.Fatalf("parsing autopsy: %v", err) + }This pattern appears in several other tests (lines 278-280, 326-328, 361-363). Consider applying the same fix consistently.
412-427: Test assertion may not validate the expected behavior.The test at lines 412-427 has unclear assertions:
- Line 414-417: The first assertion checks
len(modules) != 1with a comment// Both start with "src", but only logs instead of failing- The actual validation happens in the second case (lines 419-426)
The first case seems to be testing that files with a common prefix (
src/) produce a single module, but the assertion logic is inverted and usest.Logfinstead oft.Errorf.🔧 Suggested fix
func TestInferModules(t *testing.T) { modules := inferModules([]string{"src/auth/jwt.go", "src/auth/session.go", "src/billing/charge.go"}) - if len(modules) != 1 { - // Both start with "src" - t.Logf("modules = %v", modules) + // All files start with "src", so we expect "src" as the common module + if len(modules) != 1 || modules[0] != "src" { + t.Errorf("expected [src], got %v", modules) }cli/internal/autopsy/autopsy.go (1)
224-240: Sort inferred modules before persisting them.
seenis a map, so the module order in the returned slice is nondeterministic. Since these JSON files may be committed, sorting here avoids needless diff churn for identical failures.♻️ Proposed fix
import ( "encoding/json" "fmt" "os" "path/filepath" + "sort" "strings" "time" ) @@ for m := range seen { modules = append(modules, m) } + sort.Strings(modules) return modules
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5001bd97-9406-4c3b-83f5-d3178bbf3ee4
📒 Files selected for processing (9)
cli/cmd/fellowship/main.gocli/internal/autopsy/autopsy.gocli/internal/autopsy/autopsy_test.gocli/internal/hooks/guard.gocli/internal/hooks/guard_test.goplugin/commands/rekindle.mdplugin/skills/fellowship/SKILL.mdplugin/skills/quest/SKILL.mdplugin/skills/retro/SKILL.md
- Log corrupt autopsy files to stderr instead of silently skipping - Skip autopsies with unparseable timestamps instead of treating as non-expired Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move AutopsyExpiryDays config reading from autopsy to datadir package - Narrow gate escape from all fellowship commands to explicit allowlist - Add tests for non-allowlisted commands blocked during gate_pending Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Pin HOME in autopsy tests to prevent custom config interference - Use existing readUserConfig pattern for AutopsyExpiryDays - Extend cfg struct with autopsy config instead of duplicate file reading Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevents false positives when both query and autopsy files have no directory component (filepath.Dir returns "."). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/internal/autopsy/autopsy.go (1)
286-308: Consider sanitizing additional characters in filenames.The
sanitizefunction only handles spaces and slashes. Characters like:,*,?,",<,>,|are invalid in Windows filenames and could cause issues if quest names contain them. This is a minor concern since quest names are typically controlled, but worth noting.♻️ Optional: Broader sanitization
func sanitize(s string) string { s = strings.ReplaceAll(s, " ", "-") s = strings.ReplaceAll(s, "/", "-") + // Additional characters problematic on Windows + for _, c := range []string{":", "*", "?", "\"", "<", ">", "|", "\\"} { + s = strings.ReplaceAll(s, c, "-") + } if len(s) > 40 { s = s[:40] } return s }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2dd261e-509b-48b2-9cfb-028504801ebe
📒 Files selected for processing (7)
cli/cmd/fellowship/main.gocli/internal/autopsy/autopsy.gocli/internal/autopsy/autopsy_test.gocli/internal/datadir/datadir.gocli/internal/datadir/datadir_test.gocli/internal/hooks/guard.gocli/internal/hooks/guard_test.go
- Add random bytes to autopsy filenames for collision resistance - Return error on failed herald read instead of silently skipping - Tighten file matching to require directory boundaries (no false positives like foo.go matching foobar.go) - Sanitize additional characters in filenames (colons, asterisks, etc.) - Check errors in test assertions instead of silently ignoring - Fix undefined zombie classification in rekindle.md (use stale) - Use pinned CLI path in rekindle.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6e846a1-ea46-4771-9a51-a471bddb919b
📒 Files selected for processing (3)
cli/internal/autopsy/autopsy.gocli/internal/autopsy/autopsy_test.goplugin/commands/rekindle.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/commands/rekindle.md
- Guard against nil CreateInput with early return - Check rand.Read() error for collision-resistant filenames - Return errors from Scan instead of silently skipping bad autopsies - Sort inferred modules for deterministic JSON output - Fix test assertions: use t.Errorf/t.Fatalf instead of t.Logf, check errors from ReadFile/Unmarshal in Infer tests - Add TestCreate_NilInput test case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
cli/internal/autopsy/autopsy_test.go (3)
144-149: Consider checkingCreatereturn error in test setup.While the test would eventually fail if
Createfails, checking the error provides clearer diagnostics.♻️ Proposed fix
- Create(repo, &CreateInput{ + _, err := Create(repo, &CreateInput{ Quest: "quest-1", Trigger: "recovery", Files: []string{"src/auth/jwt.go"}, WhatFailed: "auth issue", }) + if err != nil { + t.Fatalf("setup Create failed: %v", err) + }This pattern applies to other Scan tests as well (lines 162, 180, 198).
254-263: Minor: Test doesn't set HOME environment variable.Unlike other tests that use
setupTestRepoor explicitly setHOME, this test uses a rawt.TempDir(). While it works because the autopsies directory doesn't exist, consider usingt.Setenv("HOME", t.TempDir())for consistency with other tests in this file.♻️ Proposed fix
func TestScan_EmptyDirectory(t *testing.T) { + t.Setenv("HOME", t.TempDir()) repo := t.TempDir() // no autopsies dir
286-286: Consider checkingtome.Saveerror in test setup.The
tome.Savecall at line 286 (and similar calls at lines 331, 381, 418) ignores the error return value. While unlikely to fail in test environments, checking errors would provide clearer diagnostics if something goes wrong.♻️ Proposed fix
- tome.Save(filepath.Join(tomeDir, "quest-tome.json"), qt) + if err := tome.Save(filepath.Join(tomeDir, "quest-tome.json"), qt); err != nil { + t.Fatalf("setup tome.Save failed: %v", err) + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2f4d49a-de13-4adc-a369-12c08f8a691c
📒 Files selected for processing (2)
cli/internal/autopsy/autopsy.gocli/internal/autopsy/autopsy_test.go
1 participant
Summary
.fellowship/autopsies/so future quests learn from past mistakesautopsy create(JSON stdin),autopsy scan(match by files/modules/tags),autopsy infer(reconstruct from worktree signals)fellowshipCLI commands through the gate, not justgate approve/reject/initautopsy inferto Gandalf's dead-quest handling and rekindle recovery flow/retroretrospective analysisautopsy.expiryDaysin~/.claude/fellowship.jsonCloses #67
Test plan
go test ./...)go build ./cmd/fellowship/)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behavior Changes
Documentation
Tests