fix(skills): stop auto-saving exploration transcripts as procedure skills

AGENTS.md

@@ -132,7 +132,7 @@ System prompt priority: `--system` flag > `~/.odek/IDENTITY.md` > compiled-in de
 Layered prompt-injection / approval-fatigue defenses. The full per-mitigation list lives in [docs/SECURITY.md](docs/SECURITY.md); `cmd/odek/security_report_validation_test.go` is the regression bar. Summary by layer:
 
 - **Untrusted-content boundary** (`cmd/odek/untrusted.go`) — every externally-sourced tool result (browser, file/shell/search tools, MCP, session_search, @-refs, --ctx, attachments) is wrapped in a per-call nonce'd `<untrusted_content_<nonce>>` tag; tool-result delimiters are also nonce'd (`internal/loop`). Skill/episode context injected into the system prompt is wrapped too. The per-session audit log (`cmd/odek/audit.go`) records every ingest and flags divergence between user-mentioned resources and agent actions.
-- **Provenance gates** — tainted memory episodes are stored but never auto-replayed; skills from untrusted sources (agent-created, project `./.odek/skills/`, LLM-suggested) are pinned `NeedsReview` until `odek skill promote --force`, excluded from trigger matching, and blocked from frontmatter edits via `skill_patch`. Skill auto-save applies recurrence, near-duplicate, secret-scan, and scope gates (project skills never promoted to global). `odek` self-invocation via shell is `system_write` so the agent can't reach its own trust mutations.
+- **Provenance gates** — tainted memory episodes are stored but never auto-replayed; skills from untrusted sources (agent-created, project `./.odek/skills/`, LLM-suggested) are pinned `NeedsReview` until `odek skill promote --force`, excluded from trigger matching, and blocked from frontmatter edits via `skill_patch`. Skill auto-save applies a substance bar (multi-step sequences dominated by read-only inspection commands, or carrying oversized/multi-line session-specific steps, are rejected as exploration transcripts), content-keyed recurrence (fingerprint = heuristic + name + normalized command-log digest), near-duplicate, secret-scan, and scope gates (project skills never promoted to global). `odek` self-invocation via shell is `system_write` so the agent can't reach its own trust mutations.
 - **Danger classifier** (`internal/danger/classifier.go`) — bypass-resistant normalization ($IFS, command substitution, wrappers, backslashes, basenames); covers awk/sed/editor escapes, pipe-fed xargs composition, root-level mutation targets, git data-loss verbs, `gh` as network egress, `git -c`/config code exec, find/rsync destructive flags, env dumps, shell operand/redirect path classification (writes to shell rc files, ~/.ssh, ~/.odek escalate to system_write). Trust anchors under `~/.odek` are write-protected from generic file tools.
 - **Approval friction** — TTY/WS/Telegram approvers engage friction after 3 same-class approvals in 60s (type `approve`, pause, trust shortcut hidden); `destructive`/`blocked`/`unknown` never get trust shortcuts. TTY prompts are process-wide serialized.
 - **Sub-agent caps** — `delegate_tasks` carries trust_level + max_risk enforced via the sub-agent's DangerousConfig; MCP tools withheld from untrusted sub-agents; API keys handed off via unlinked-tempfile FD, never env.

cmd/odek/main_test.go

@@ -1525,7 +1525,9 @@ func TestParseReplFlags_ExtraArgsIgnored(t *testing.T) {
 
 // multiTurnServer returns an httptest server that simulates a multi-turn
 // conversation: n terminal tool calls followed by a final text response.
-// Each tool call executes echo step N (safe, no side effects).
+// Each tool call executes `true step N` — exits 0 with no side effects, and
+// is an action verb so the multi-step substance bar lets the sequence
+// through (a pure-inspection sequence like `echo ...` is now rejected).
 // Handles /models discovery requests from llm.DiscoverModelContext.
 func multiTurnServer(t *testing.T, terminalCalls int) *httptest.Server {
 	t.Helper()
@@ -1542,7 +1544,7 @@ func multiTurnServer(t *testing.T, terminalCalls int) *httptest.Server {
 		callCount++
 		w.Header().Set("Content-Type", "application/json")
 		if callCount <= terminalCalls {
-			fmt.Fprintf(w, `{"choices":[{"message":{"content":"Running step %d.","tool_calls":[{"id":"call_%d","function":{"name":"shell","arguments":"{\"command\":\"echo step %d\"}"}}]}}]}`,
+			fmt.Fprintf(w, `{"choices":[{"message":{"content":"Running step %d.","tool_calls":[{"id":"call_%d","function":{"name":"shell","arguments":"{\"command\":\"true step %d\"}"}}]}}]}`,
 				callCount, callCount, callCount)
 		} else {
 			w.Write([]byte(`{"choices":[{"message":{"content":"All steps completed successfully."}}]}`))
@@ -1608,7 +1610,7 @@ func TestRunLearn_MultiStepProcedure(t *testing.T) {
 	}
 
 	// Skill file written to disk — poll since the goroutine may still be writing.
-	skillDir := filepath.Join(homeDir, ".odek", "skills", "procedure-echo")
+	skillDir := filepath.Join(homeDir, ".odek", "skills", "procedure-true")
 	skillFile := filepath.Join(skillDir, "SKILL.md")
 	for i := 0; i < 10; i++ {
 		if _, err := os.Stat(skillFile); err == nil {
@@ -1678,7 +1680,7 @@ func TestRunLearn_InteractiveReject(t *testing.T) {
 	}
 
 	// Verify no skill file written
-	skillDir := filepath.Join(homeDir, ".odek", "skills", "procedure-echo")
+	skillDir := filepath.Join(homeDir, ".odek", "skills", "procedure-true")
 	skillFile := filepath.Join(skillDir, "SKILL.md")
 	if _, err := os.Stat(skillFile); !os.IsNotExist(err) {
 		t.Errorf("skill file should NOT exist after rejection: %s", skillFile)

docs/LEARNING.md

@@ -60,7 +60,7 @@ odek run --learn "set up CI with GitHub Actions"
 - **Learning is non-blocking** — skill detection and auto-save run in a background goroutine after the agent's response is delivered. The process exits immediately; learning completes asynchronously on a best-effort basis.
 - **Tainted skills require explicit promotion** — skills learned from `browser`, MCP tools, or sensitive file reads are saved with `Provenance.Untrusted=true` and `NeedsReview=true`. They cannot be auto-loaded until you run `odek skill promote <name> --force` after reviewing the body.
 - **Scope gates keep garbage out** — machine-specific suggestions (absolute home-directory paths) are dropped entirely. Project-specific suggestions (repo-rooted `./scripts/...` invocations, hardcoded release version tags) are redirected to the project skills dir `./.odek/skills` instead of the global `~/.odek/skills` — project-related skills are never promoted to global, and project-dir skills stay pinned to `NeedsReview` until promoted locally. Auto-curation (merge/prune) is confined to the global dir, so project skills can never be merged into or deleted by the global curator.
-- **Recurrence before persistence** — a pattern must be seen in at least `auto_save.min_occurrences` distinct sessions (default 2) before it is eligible for auto-save; first sightings are recorded in `~/.odek/skills/.candidates.json` and reported as pending.
+- **Recurrence before persistence** — a pattern must be seen in at least `auto_save.min_occurrences` distinct sessions (default 2) before it is eligible for auto-save; first sightings are recorded in `~/.odek/skills/.candidates.json` and reported as pending. The recurrence fingerprint keys on heuristic + name + a digest of the normalized command log, so two unrelated sessions that merely share a generic name (`procedure-ls`) do not count as a recurrence.
 - **Save-time hygiene** — suggestions are score-ranked (verification section, command-log evidence, LLM-judged) so the `max_per_run` budget goes to the strongest candidates; near-duplicates of existing skills (Jaccard word-set similarity ≥ 0.85) are skipped instead of creating parallel skills; and every SKILL.md write is secret-scanned, with detected credentials replaced by `[REDACTED]` and the skill pinned to `NeedsReview`.
 - **LLM curation** — with `llm_curate` enabled, merge bodies are synthesized by the model (deduplicated, coherent) instead of mechanically concatenated; failures fall back to concatenation.
 
@@ -167,6 +167,10 @@ Detects **4 or more sequential successful terminal calls**. Failed commands brea
 - 4+ consecutive `shell` tool calls that all succeeded (no `error:` in output)
 - Non-terminal tools (read_file, write_file, etc.) are skipped — they don't count but don't break the sequence
 
+**Substance bar (keeps exploration transcripts out):**
+- Every step must be a short single-line command (≤ 200 chars) — a step carrying a session-specific payload (full commit message, heredoc) disqualifies the whole sequence.
+- Read-only inspection commands (`ls`, `grep`, `cat`, `head`, `sed -n`, `echo`, `git status/log/diff`, `go test/vet`, …) must not dominate: if more than half the steps only observe state, the sequence was exploration, not a reusable procedure, and no suggestion is made.
+
 **Example:**
 ```bash
 git clone https://github.com/example/repo.git   # step 1

internal/skills/candidates.go

@@ -1,9 +1,12 @@
 package skills
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 )
 
@@ -13,9 +16,12 @@ import (
 // CandidateStore persists how often each suggestion fingerprint has been
 // seen across sessions (<userDir>/.candidates.json); a suggestion is only
 // eligible for saving once its count reaches
-// AutoSaveConfig.MinOccurrences. Suggestion names are deterministic per
-// pattern ("corrected-git", "procedure-docker", ...), so heuristic+name is
-// a stable fingerprint even though body text varies between sessions.
+// AutoSaveConfig.MinOccurrences. The fingerprint keys on heuristic, name,
+// AND a digest of the normalized command log — keying on the name alone
+// let unrelated sessions collide ("procedure-ls" fires in any session that
+// starts with `ls`), so the recurrence gate passed without the pattern
+// actually recurring. With the content digest, only the same normalized
+// command pattern seen again counts as a recurrence.
 
 // CandidateFileName is the store file inside the skills user dir.
 const CandidateFileName = ".candidates.json"
@@ -36,9 +42,17 @@ type CandidateStore struct {
 	Candidates map[string]CandidateEntry `json:"candidates"`
 }
 
-// candidateFingerprint identifies a suggestion across sessions.
+// candidateFingerprint identifies a suggestion across sessions: heuristic
+// and name, plus a short digest of the normalized command log so that two
+// unrelated sessions sharing only a generic name ("procedure-ls") do not
+// count as the same pattern recurring.
 func candidateFingerprint(s SkillSuggestion) string {
-	return s.Heuristic + "|" + s.Name
+	norm := make([]string, 0, len(s.CommandLog))
+	for _, c := range s.CommandLog {
+		norm = append(norm, normalizeCommand(c))
+	}
+	sum := sha256.Sum256([]byte(strings.Join(norm, "\n")))
+	return s.Heuristic + "|" + s.Name + "|" + hex.EncodeToString(sum[:])[:12]
 }
 
 // LoadCandidates reads the candidate store from disk. A missing or

internal/skills/candidates_test.go

@@ -64,6 +64,47 @@ func TestAutoSaveSuggestions_RecurrenceDisabled(t *testing.T) {
 	}
 }
 
+// TestCandidateFingerprint_KeysOnCommandLog pins the fix for generic-name
+// collisions: two sessions that both yield "procedure-ls" but ran
+// different commands are NOT the same pattern recurring, so the gate must
+// keep them pending forever.
+func TestCandidateFingerprint_KeysOnCommandLog(t *testing.T) {
+	a := SkillSuggestion{Name: "procedure-ls", Heuristic: "multi-step", CommandLog: []string{"ls -d cmd/*", "go test ./...", "git status", "make build"}}
+	b := SkillSuggestion{Name: "procedure-ls", Heuristic: "multi-step", CommandLog: []string{"ls -la", "grep -rn foo .", "git log", "git push"}}
+	if candidateFingerprint(a) == candidateFingerprint(b) {
+		t.Error("same generic name with different command logs must not share a fingerprint")
+	}
+	if candidateFingerprint(a) != candidateFingerprint(a) {
+		t.Error("identical suggestion must have a stable fingerprint")
+	}
+}
+
+// TestAutoSaveSuggestions_NoFalseRecurrence: the same generic suggestion
+// name from two unrelated sessions must not reach MinOccurrences.
+func TestAutoSaveSuggestions_NoFalseRecurrence(t *testing.T) {
+	dir := t.TempDir()
+	cfg := DefaultSkillsConfig() // MinOccurrences: 2
+	cfg.AutoSave.MaxPerRun = 5
+
+	session1 := []SkillSuggestion{
+		{Name: "procedure-ls", Heuristic: "multi-step", Body: recurringBody, CommandLog: []string{"ls -d cmd/*", "go build ./...", "git status", "make test"}},
+	}
+	session2 := []SkillSuggestion{
+		{Name: "procedure-ls", Heuristic: "multi-step", Body: recurringBody, CommandLog: []string{"ls -la /tmp", "go build ./cmd/app", "git diff", "make lint"}},
+	}
+
+	if r := AutoSaveSuggestions(session1, dir, "", cfg, nil, guard.Config{}, false); len(r.Pending) != 1 {
+		t.Fatalf("session 1 should pend, got %+v", r)
+	}
+	r := AutoSaveSuggestions(session2, dir, "", cfg, nil, guard.Config{}, false)
+	if len(r.Saved) != 0 {
+		t.Errorf("unrelated sessions sharing a generic name must not trigger recurrence, saved %v", r.Saved)
+	}
+	if len(r.Pending) != 1 {
+		t.Errorf("session 2 should also pend as a distinct pattern, got %+v", r)
+	}
+}
+
 // TestCandidateStore_PrunesStaleEntries covers the age-based pruning that
 // bounds the store file.
 func TestCandidateStore_PrunesStaleEntries(t *testing.T) {

internal/skills/selfimprove.go

@@ -334,6 +334,9 @@ func buildSuggestionFromSequence(seq []ToolCall, heuristic string) *SkillSuggest
 	if len(seq) < 4 {
 		return nil
 	}
+	if !isReusableProcedure(seq) {
+		return nil
+	}
 
 	topic := extractTopic(seq[0].Input)
 	var steps []string
@@ -350,6 +353,122 @@ func buildSuggestionFromSequence(seq []ToolCall, heuristic string) *SkillSuggest
 	}
 }
 
+// maxProcedureStepLen caps one step's command length. Longer commands are
+// session-specific payloads (full commit messages, heredocs, one-off
+// pipelines) that can never be replayed as written.
+const maxProcedureStepLen = 200
+
+// inspectionVerbs are read-only commands: they observe state but change
+// nothing. A sequence dominated by them is an exploration transcript, not
+// a reusable procedure.
+var inspectionVerbs = map[string]bool{
+	"ls": true, "cat": true, "head": true, "tail": true, "grep": true,
+	"find": true, "wc": true, "echo": true, "printf": true, "pwd": true,
+	"file": true, "stat": true, "tree": true, "which": true, "env": true,
+	"gofmt": true, "sort": true, "uniq": true, "diff": true, "jq": true,
+	"less": true, "more": true,
+}
+
+// gitReadOnlySubcommands are the git verbs that only observe repository
+// state; every other subcommand (commit, push, tag, add, ...) mutates and
+// counts as an action.
+var gitReadOnlySubcommands = map[string]bool{
+	"status": true, "log": true, "diff": true, "show": true, "blame": true,
+	"describe": true, "rev-parse": true, "ls-files": true, "grep": true,
+	"shortlog": true, "branch": true, "remote": true,
+}
+
+// isInspectionCommand reports whether a command only observes state.
+// Pipelines and separators are judged by their lead command — `grep foo
+// file | head` is inspection, `go test ./... | tail` is an action.
+func isInspectionCommand(cmd string) bool {
+	verb := leadVerb(cmd)
+	switch verb {
+	case "git":
+		return gitLeadSubcommandIsReadOnly(cmd)
+	case "go":
+		// `go test/vet` verify; `go build/install/run` produce effects.
+		// Neither dominates a real procedure on its own, but test/vet are
+		// the verbs that pad exploration transcripts, so count them as
+		// inspection.
+		sub := secondToken(cmd)
+		return sub == "test" || sub == "vet" || sub == "list" || sub == "env" || sub == "doc"
+	case "sed", "awk":
+		// Without -i, sed/awk only print.
+		return !strings.Contains(cmd, "-i")
+	}
+	return inspectionVerbs[verb]
+}
+
+// secondToken returns the first non-flag token after the lead verb, or "".
+func secondToken(cmd string) string {
+	fields := strings.Fields(strings.TrimSpace(cmd))
+	seenVerb := false
+	for _, f := range fields {
+		if isPlumbingToken(f) {
+			continue
+		}
+		if !seenVerb {
+			seenVerb = true
+			continue
+		}
+		if strings.HasPrefix(f, "-") {
+			continue
+		}
+		return strings.Trim(f, "\"'`")
+	}
+	return ""
+}
+
+// gitLeadSubcommandIsReadOnly extracts the subcommand of the leading `git`
+// invocation (skipping global flags and their values) and reports whether
+// it only reads state. Unrecognized subcommands count as actions — a
+// conservative default, since unknown git verbs usually mutate.
+func gitLeadSubcommandIsReadOnly(cmd string) bool {
+	fields := strings.Fields(strings.TrimSpace(cmd))
+	seenGit := false
+	skipNext := false
+	for _, f := range fields {
+		if !seenGit {
+			if f == "git" {
+				seenGit = true
+			}
+			continue
+		}
+		if skipNext {
+			skipNext = false
+			continue
+		}
+		if strings.HasPrefix(f, "-") {
+			// Global flags that take a separate value (-C dir, -c k=v).
+			if f == "-C" || f == "-c" {
+				skipNext = true
+			}
+			continue
+		}
+		return gitReadOnlySubcommands[strings.Trim(f, "\"'`")]
+	}
+	return false
+}
+
+// isReusableProcedure applies the substance bar to a candidate sequence:
+// every step must be a short single-line command (no embedded transcripts),
+// and read-only inspection commands must not dominate — a session that was
+// mostly `ls`/`grep`/`sed -n` exploration is not a reusable procedure, no
+// matter how often its first verb recurs.
+func isReusableProcedure(seq []ToolCall) bool {
+	inspection := 0
+	for _, c := range seq {
+		if strings.Contains(c.Input, "\n") || len(c.Input) > maxProcedureStepLen {
+			return false
+		}
+		if isInspectionCommand(c.Input) {
+			inspection++
+		}
+	}
+	return inspection*2 <= len(seq)
+}
+
 func generateProcedureBody(topic string, steps []string) string {
 	var b strings.Builder
 	fmt.Fprintf(&b, "## Overview\n\nProcedure for: %s\n\n", topic)