v2.10.0: memory across runs and learning PRs

CHANGELOG.md

@@ -1,5 +1,64 @@
 # Changelog
 
+## v2.10.0
+
+Memory across runs. A run's lessons now survive it: a read-only retro stage proposes at
+most one lesson or skill edit after an issue's final outcome, and `factory learn` batches
+whatever's pending into a PR you review, never merges automatically.
+
+- **`research/agents/claude/memory.md`.** Headless Claude's own auto-memory writes to a
+  path keyed by a hash of the worktree's absolute directory, under the operator's home
+  directory: operator-machine-local state, gone once a worktree is torn down, invisible in
+  CI, and never shared across machines. Confirms the roadmap's pre-decided design: the
+  factory never relies on `~/.claude`.
+- **`src/context.ts`: repo-local memory.** `.factory/memory/lessons.md` is capped at 8 KiB
+  (`MAX_LESSONS_BYTES`), keeping the most recent lines and dropping the oldest when it
+  would grow past the cap, and is injected into every stage's context pack the same way on
+  the Mac, a VPS or in CI.
+- **`src/watch.ts`: a retro stage after each final outcome** (merged, rejected by you, or
+  given up on after the third verify rejection past `MAX_VERIFY_REJECTS`), plus a fourth
+  trigger from the dashboard's operator-merge route, which queues the retro for `pollOnce`
+  to drain since it has no `Executor` of its own to run it. `runRetro` never goes through
+  `runStage`'s `upsertRun` (an already-terminal run would be put back to "running"), but
+  its cost still rolls into spend-cap accounting through the same `recordStageRun` call.
+  Read-only, on Haiku by default; it proposes at most one lesson or skill edit, or none
+  (neo's rule of at most one task per review, idea only, no code copied), and writes a
+  `retro.json` artifact linking the run.
+- **`src/learn.ts`: `factory learn`.** A deterministic, non-agent command that batches
+  every pending retro proposal into `.factory/memory/lessons.md` and per-skill
+  `.claude/skills/<name>/PROPOSED_EDITS.md` files, commits, and diffs the branch against
+  base before pushing. `outsideAllowedPaths` (`src/boundary.ts`) is the one exemption from
+  `protectedPaths`, and only for a `factory/learning-YYYYMMDD` branch: it may touch
+  `.factory/memory/**` and `.claude/skills/**`, nothing else. A row is marked learned only
+  after its branch pushes and its PR exists, so a refusal or a crash first leaves it
+  pending for the next run rather than dropping it. Idempotent per day: a second run
+  reuses the same branch and PR. It never auto-merges, and shows in the dashboard inbox as
+  its own read-only `learning-pr` kind, since it has no linked issue to chatops against.
+- **`src/boundary.ts`: `touchesProtectedPath` now always refuses `.claude/**` and
+  `.factory/**`, merged inside the function itself rather than at each call site.** A
+  normal build's push check (`src/watch.ts`) relied entirely on the repo's own
+  `protectedPaths`, which defaults to `[]`; `guard-paths.sh` already hardcoded both paths
+  unconditionally for interactive edits, so a Bash-made edit outside that hook could still
+  reach a push. Baking the merge into `touchesProtectedPath` itself, rather than patching
+  the build-stage call site alone, also closes the same gap at `src/merge-policy.ts`'s
+  `autoEligible`, a second call site the first fix missed.
+
+Structural tests: a learning PR outside its allowed paths is refused before any push
+(`tests/learn.test.ts`); the lessons file cap holds (`tests/context-pack.test.ts`); a
+normal build still cannot touch `.claude/**` with the default empty `protectedPaths`
+(`tests/scenarios.test.ts`), and neither can an auto-merge-eligible PR
+(`tests/boundary.test.ts`, covering both `touchesProtectedPath` call sites at once).
+
+Not in v2.10.0:
+- A live run of the retro stage and `factory learn` against splitbill-demo: the
+  currently-installed production `factory` predates this branch, so a genuine run needs a
+  reset to a clean baseline first, the same category of blocker as v2.7.0's live run,
+  pending your reset OK.
+- Mutual-exclusion enforcement between a lesson and a skill edit on one retro row: the
+  retro skill's own instructions say "propose at most one," but nothing in `src/schemas.ts`
+  or `src/learn.ts` would refuse a row carrying both. Left as a prompt-level rule, since no
+  run has produced one yet.
+
 ## v2.9.0
 
 The review inbox stops sending you to GitHub. A PR waiting on you now shows its diff, gate

bin/factory

@@ -18,7 +18,7 @@ import { BinaryRunner, ClaudeRechecker } from "../src/recheck";
 import { scan } from "../src/scan";
 import { streamLogs } from "../src/logs";
 import { plain } from "../src/display";
-import { act, buildInbox, inboxPositionals, type InboxAction } from "../src/inbox";
+import { act, buildInbox, inboxPositionals, learningPrItems, type InboxAction } from "../src/inbox";
 import { reset, rebaseline } from "../src/reset";
 import { runDoctor, fixDoctor } from "../src/doctor";
 import { loadMachineConfig, MachineLeases, MachineSpend } from "../src/machine";
@@ -30,6 +30,7 @@ import { ensureRepoClone } from "../src/repo";
 import { helpText } from "../src/help";
 import { FixtureRecorder } from "../src/agents/record";
 import { configFor, formatReport, reportFor } from "../src/verify-agent";
+import { runLearn } from "../src/learn";
 
 const args = process.argv.slice(2);
 const command = args[0];
@@ -182,7 +183,7 @@ async function cmdInbox(): Promise<void> {
   const repo = flag("repo") ?? process.env.FACTORY_REPO;
   if (!repo) throw new UsageError("inbox: --repo <owner/name> (or FACTORY_REPO) is required");
   const github = new GitHub();
-  const items = buildInbox(await github.listOpenIssues(repo));
+  const items = [...buildInbox(await github.listOpenIssues(repo)), ...learningPrItems(await github.listPrs(repo, { state: "open" }))];
   const [issueArg, actionArg] = inboxPositionals(args);
   if (issueArg) {
     const item = items.find((i) => i.issue === Number(issueArg));
@@ -400,6 +401,22 @@ async function cmdVerifyAgent(): Promise<void> {
   if (!report.pass) process.exit(EXIT.runFailed);
 }
 
+async function cmdLearn(): Promise<void> {
+  const cloneDir = await resolveCloneDir();
+  const config = await loadConfig(cloneDir);
+  const deps = buildWatchDeps(cloneDir, config);
+  const result = await runLearn(deps, config.repo, config.base);
+  if (has("json")) {
+    console.log(successJson(result, true));
+    return;
+  }
+  if (result.batched === 0) {
+    console.log("factory learn: nothing pending.");
+    return;
+  }
+  console.log(`factory learn: batched ${result.batched} proposal(s) on ${result.branch} -> ${result.prUrl}`);
+}
+
 async function cmdInstall(): Promise<void> {
   const target = args[1];
   if (!target) throw new UsageError("install: usage: factory install <target-dir> [--dry-run] [--update] [--ci] [--agents a,b,c]");
@@ -445,6 +462,8 @@ async function main(): Promise<void> {
       return cmdInstall();
     case "verify-agent":
       return cmdVerifyAgent();
+    case "learn":
+      return cmdLearn();
     default:
       console.log(helpText());
       if (command && !["help", "--help", "-h"].includes(command)) process.exit(EXIT.error);

dashboard/server.ts

@@ -15,7 +15,7 @@ import { FactoryState, DEFAULT_DB_PATH } from "../src/state";
 import { parseChatOps } from "../src/chatops";
 import { buildBoard } from "./board";
 import { LABEL } from "../src/labels";
-import { InboxError, act, buildInbox, type InboxAction } from "../src/inbox";
+import { InboxError, act, buildInbox, learningPrItems, type InboxAction } from "../src/inbox";
 import { plain } from "../src/display";
 import { agentCatalog } from "../src/agents/docs";
 import { agentChecks } from "../src/doctor";
@@ -88,6 +88,8 @@ export function createDashboard(state: FactoryState, github: GitHub, repo: strin
   const sessions = new Map<string, number>();
   const issuesCache = new Map<string, { at: number; issues: Awaited<ReturnType<GitHub["listOpenIssues"]>> }>();
   const issuesInflight = new Map<string, Promise<Awaited<ReturnType<GitHub["listOpenIssues"]>>>>();
+  const prsCache = new Map<string, { at: number; prs: Awaited<ReturnType<GitHub["listPrs"]>> }>();
+  const prsInflight = new Map<string, Promise<Awaited<ReturnType<GitHub["listPrs"]>>>>();
 
   // The Agents page shows each configured agent's installed version and doctor rows.
   // Probing spawns `--version`, so it is cached for a minute and shared by concurrent requests.
@@ -145,6 +147,30 @@ export function createDashboard(state: FactoryState, github: GitHub, repo: strin
     issuesCache.delete(r);
   }
 
+  // Same single-flight + cache pattern as cachedIssuesFor, for the inbox's
+  // learning-PR items (plan v2.10.0 item 4), which come from `gh pr list`,
+  // not issues.
+  async function cachedPrsFor(r: string) {
+    const now = Date.now();
+    const cached = prsCache.get(r);
+    if (cached && now - cached.at < BOARD_CACHE_MS) return cached.prs;
+    const inflight = prsInflight.get(r);
+    if (inflight) return inflight;
+    const promise = github
+      .listPrs(r, { state: "open" })
+      .then((prs) => {
+        prsCache.set(r, { at: Date.now(), prs });
+        prsInflight.delete(r);
+        return prs;
+      })
+      .catch((err) => {
+        prsInflight.delete(r);
+        throw err;
+      });
+    prsInflight.set(r, promise);
+    return promise;
+  }
+
   async function cachedIssues() {
     return cachedIssuesFor(repo);
   }
@@ -360,7 +386,13 @@ export function createDashboard(state: FactoryState, github: GitHub, repo: strin
         const decision = decideOperatorMerge(readiness, ci);
         const merged = await attemptMerge(github, r, pr.number, decision);
         await github.commentIssue(r, issueNumber, renderOperatorAuditComment(decision));
-        if (merged) invalidateIssuesCache(r);
+        // The dashboard has no Executor to run retro itself (plan v2.10.0 item
+        // 3); queueRetro just records the row, and watch.ts's poll loop
+        // (runQueuedRetros) drains it on its next tick.
+        if (merged) {
+          invalidateIssuesCache(r);
+          state.queueRetro(r, issueNumber, "merged");
+        }
         return json({ ok: merged, decision });
       },
     },
@@ -425,7 +457,12 @@ export function createDashboard(state: FactoryState, github: GitHub, repo: strin
         const filter = url.searchParams.get("repo");
         const repos = filter ? [filter] : inboxRepos();
         if (repos.length === 0) return json({ error: "no repo configured or discovered" }, { status: 500 });
-        const perRepo = await Promise.all(repos.map(async (r) => buildInbox(await cachedIssuesFor(r)).map((i) => ({ ...i, repo: r }))));
+        const perRepo = await Promise.all(
+          repos.map(async (r) => [
+            ...buildInbox(await cachedIssuesFor(r)).map((i) => ({ ...i, repo: r })),
+            ...learningPrItems(await cachedPrsFor(r)).map((i) => ({ ...i, repo: r })),
+          ]),
+        );
         const items = perRepo.flat().sort((a, b) => (a.waitingSince ?? "").localeCompare(b.waitingSince ?? "") || a.issue - b.issue);
         return json({ repos: inboxRepos(), repo, items });
       },

install.sh

@@ -104,8 +104,8 @@ while IFS= read -r -d '' file; do
     continue
   fi
 
-  # Repo-owned files: the charter and config example are the repo's to edit.
-  if [[ "$rel" == ".factory/charter.md" || "$rel" == ".factory/config.example.json" ]] && [[ -e "$dest" || -L "$dest" ]]; then
+  # Repo-owned files: the charter, config example and accumulated lessons are the repo's to edit.
+  if [[ "$rel" == ".factory/charter.md" || "$rel" == ".factory/config.example.json" || "$rel" == ".factory/memory/lessons.md" ]] && [[ -e "$dest" || -L "$dest" ]]; then
     echo "skip (repo-owned): $rel"
     skipped=$((skipped + 1))
     continue

package.json

@@ -1,6 +1,6 @@
 {
   "name": "software-factory",
-  "version": "2.9.0",
+  "version": "2.10.0",
   "private": true,
   "type": "module",
   "description": "GitHub-native SDLC loop for coding agents: triage, plan, build, verify, PR.",

research/agents/claude/memory.md

@@ -0,0 +1,59 @@
+# Headless Claude auto-memory, under the factory's own flags
+
+v2.10.0 item 1. Answers where headless Claude reads and writes auto-memory when invoked
+with the factory's exact flags, and whether the factory can rely on it. It cannot; this
+is why item 2 puts memory in the repo instead.
+
+## Test
+
+Local `claude` binary, v2.1.283. From a fresh, throwaway `git init` repo (no
+`.claude/`, no `CLAUDE.md`), ran the same flags `claudeArgs` builds in
+`src/agents/presets/claude.ts`:
+
+```
+claude -p "Reply with exactly one line: MEMORY_INSTRUCTIONS=yes if your system prompt or
+context mentions any auto-memory system, a memory directory, RTK, or engineering.md;
+otherwise MEMORY_INSTRUCTIONS=no. Do not use any tools." \
+  --output-format stream-json --verbose \
+  --setting-sources project,local \
+  --no-session-persistence \
+  --permission-mode dontAsk
+```
+
+`--setting-sources project,local` deliberately excludes `user`; `--no-session-persistence`
+should mean no transcript is kept.
+
+## Result
+
+- The model answered `MEMORY_INSTRUCTIONS=yes`: it saw the invoking user's global
+  `~/.claude/CLAUDE.md` (RTK, engineering.md) despite `user` being excluded from
+  `--setting-sources`.
+- The CLI's own `system/init` event carried a live path:
+  `"memory_paths":{"auto":"/Users/param/.claude/projects/<hash-of-cwd>/memory/"}`,
+  where `<hash-of-cwd>` encodes the probe's absolute working directory.
+- That directory exists on disk (created by the CLI on startup) but is empty: this run
+  wrote nothing into it, only reserved the path.
+
+## What this means
+
+- **`--setting-sources` gates settings files, not the auto-memory/global-instructions
+  mechanism.** That mechanism sits outside the user/project/local settings hierarchy and
+  stayed active even with `user` excluded and session persistence off.
+- **The memory path is keyed by a hash of the absolute working directory**, under the
+  *invoking user's own* `~/.claude/projects/`. Two consequences for the factory:
+  - A worktree at `factory/.worktrees/issue-N` gets its own distinct memory directory,
+    unrelated to any other worktree's, the main checkout's, or a prior run's for the same
+    issue once the worktree path changes. Nothing coalesces.
+  - The directory lives under the operator's home directory. It is not in the repo, not
+    portable to a different machine (a VPS runner), and not visible in CI.
+- This is operator-machine-local state, not repo state. It cannot be the factory's memory
+  mechanism: it cannot be read back by a run on a different machine, doesn't survive a
+  worktree being torn down and recreated under a fresh path, and isn't inspectable in a PR
+  diff for review.
+
+## Conclusion
+
+Confirms the roadmap's pre-decided design: **the factory never relies on `~/.claude`**.
+Persistent lessons must live in the repo itself, at `.factory/memory/lessons.md`
+(v2.10.0 item 2), injected through `buildContextPack` (`src/context.ts`) the same way for
+every machine, worktree and CI run alike.

src/agents/docs.ts

@@ -36,7 +36,7 @@ export function renderAgentsDoc(doc: string): string {
   return `${doc.slice(0, start + TABLE_START.length)}\n${agentsTable()}\n${doc.slice(end)}`;
 }
 
-const STAGE_NAMES = ["triage", "plan", "build", "verify", "pr"] as const;
+const STAGE_NAMES = ["triage", "plan", "build", "verify", "pr", "retro"] as const;
 
 export interface AgentRow {
   readonly name: string;

src/artifacts.ts

@@ -80,6 +80,17 @@ export interface VerdictArtifact {
   readonly criteria?: Criterion[];
 }
 
+// v2.10.0 item 3: read-only, runs after a final outcome. At most one proposal
+// (neo's rule of at most one task per review); all three fields absent means
+// "nothing worth changing".
+export interface RetroArtifact {
+  readonly outcome?: StepOutcome;
+  readonly summary?: string;
+  readonly lesson?: string;
+  readonly skill_name?: string;
+  readonly skill_edit?: string;
+}
+
 // A step result is one JSON object of at most 16 KiB (machinist workflow.go).
 export const MAX_STEP_JSON_BYTES = 16 * 1024;
 export const VERDICT_KEY_LIST = ["result", "rounds", "findings", "criteria", "outcome", "summary"] as const;
@@ -106,6 +117,7 @@ export const STEP_KEYS: Record<Exclude<ArtifactStage, "verify">, readonly string
   plan: ["status", "risk", "revision", "files", "autoApproveEligible", "commentId", "proof", "outcome", "summary"],
   build: ["status", "gate_line", "rounds", "outcome", "summary"],
   pr: ["outcome", "summary"],
+  retro: ["lesson", "skill_name", "skill_edit", "outcome", "summary"],
 };
 
 // `types` bounds triage's `type` enum to the repo's actual type list
@@ -173,7 +185,7 @@ export function validateVerdict(raw: unknown): { ok: true; verdict: VerdictArtif
   return { ok: true, verdict };
 }
 
-export type ArtifactStage = "triage" | "plan" | "build" | "verify" | "pr";
+export type ArtifactStage = "triage" | "plan" | "build" | "verify" | "pr" | "retro";
 
 export function runDir(issue: number): string {
   return `.factory/runs/issue-${issue}`;
@@ -188,6 +200,7 @@ export const COMMENT_FILENAMES: Record<ArtifactStage, string> = {
   build: "status-comment.md",
   verify: "verdict-comment.md",
   pr: "pr-body.md",
+  retro: "retro-comment.md",
 };
 
 export const JSON_FILENAMES: Record<ArtifactStage, string> = {
@@ -196,6 +209,7 @@ export const JSON_FILENAMES: Record<ArtifactStage, string> = {
   build: "build.json",
   verify: "verdict.json",
   pr: "pr.json",
+  retro: "retro.json",
 };
 
 // Undefined for a missing file, and also for one that is too big, not JSON, or

src/boundary.ts

@@ -4,10 +4,22 @@
 // independently diffs the branch against its base and refuses to ship if a
 // protected path was touched by any means.
 
+// Hardcoded regardless of repo config, mirroring guard-paths.sh's own
+// unconditional additions (template/.claude/hooks/guard-paths.sh): a normal
+// build must never touch the factory's own state or skills, even with the
+// default empty protectedPaths. Merged inside touchesProtectedPath itself,
+// not at each call site, so a new caller can never forget it (one resolver
+// per concept: this repo already has two call sites, src/watch.ts and
+// src/merge-policy.ts). `factory learn`'s branch never calls this check (it
+// uses outsideAllowedPaths instead), so its exemption never weakens this for
+// a normal build.
+export const ALWAYS_PROTECTED_PATHS = [".claude/**", ".factory/**"] as const;
+
 export function touchesProtectedPath(changedFiles: readonly string[], protectedPaths: readonly string[]): string[] {
+  const patterns = [...protectedPaths, ...ALWAYS_PROTECTED_PATHS];
   const hits: string[] = [];
   for (const file of changedFiles) {
-    for (const pattern of protectedPaths) {
+    for (const pattern of patterns) {
       if (new Bun.Glob(pattern).match(file)) {
         hits.push(file);
         break;
@@ -16,3 +28,16 @@ export function touchesProtectedPath(changedFiles: readonly string[], protectedP
   }
   return hits;
 }
+
+// Inverse check for `factory learn` (plan v2.10.0 item 4): a learning PR is
+// exempt from protectedPaths but is only ever allowed to touch its own two
+// globs, so this refuses anything outside them rather than checking for a hit.
+export function outsideAllowedPaths(changedFiles: readonly string[], allowedPaths: readonly string[]): string[] {
+  const misses: string[] = [];
+  for (const file of changedFiles) {
+    if (!allowedPaths.some((pattern) => new Bun.Glob(pattern).match(file))) {
+      misses.push(file);
+    }
+  }
+  return misses;
+}