fix(backlinks): honor positional check-backlinks directory argument

The help text (gbrain check-backlinks <check|fix> [dir]) promised a
positional directory argument, but runBacklinks only parsed --dir and
defaulted to cwd, so the walker ran from the wrong root and could hit
EPERM on unreadable sibling dirs.

Extract parseBacklinksArgs: positional [dir] is now honored, --dir still
overrides it, --dry-run preserved, and a --dir flag missing its value
falls back to the positional dir instead of picking up undefined.

Takeover of #852 (rebased onto master past the findBacklinkGaps dedupe
test block). Fixes #485.

Co-authored-by: Kage18 <Kage18@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs/tutorials/improving-skills-with-skillopt.md

@@ -233,14 +233,13 @@ keep it or `git checkout` to throw it away. Nothing is committed for you.
 
 **For a skill that ships with gbrain** (anything under the gbrain repo's own
 `skills/`): SkillOpt refuses to overwrite it by default and writes the winner to
-`skills/<name>/skillopt/proposed.md` instead (while keeping `best.md` as the
-optimizer's current-best pointer), so an optimization pass can never silently
-mutate a skill other people depend on. Two ways to handle that:
+`skills/<name>/skillopt/best.md` instead, so an optimization pass can never
+silently mutate a skill other people depend on. Two ways to handle that:
 
 ```bash
 # See the proposed improvement without touching SKILL.md (works for ANY skill):
 gbrain skillopt meeting-prep --split 1:1:1 --no-mutate
-# → writes skills/meeting-prep/skillopt/proposed.md, updates best.md, and prints the proposal path.
+# → writes skills/meeting-prep/skillopt/best.md (the proposed rewrite), prints its path. Copy what you want.
 
 # Actually rewrite a bundled skill (explicit opt-in + an independent held-out set):
 gbrain skillopt brain-ops --split 1:1:1 --allow-mutate-bundled \

src/commands/backlinks.ts

@@ -5,8 +5,8 @@
  * checks if back-links exist, and optionally creates them.
  *
  * Usage:
- *   gbrain check-backlinks check [--dir <brain-dir>]     # report missing back-links
- *   gbrain check-backlinks fix [--dir <brain-dir>]        # create missing back-links
+ *   gbrain check-backlinks check [dir] [--dir <brain-dir>] # report missing back-links
+ *   gbrain check-backlinks fix [dir] [--dir <brain-dir>]   # create missing back-links
  *   gbrain check-backlinks fix --dry-run                  # preview fixes
  */
 
@@ -201,6 +201,40 @@ export interface BacklinksResult {
   dryRun: boolean;
 }
 
+export interface ParsedBacklinksArgs {
+  subcommand: string | undefined;
+  brainDir: string;
+  dryRun: boolean;
+}
+
+export function parseBacklinksArgs(args: string[]): ParsedBacklinksArgs {
+  const subcommand = args[0];
+  const dryRun = args.includes('--dry-run');
+  const dirIdx = args.indexOf('--dir');
+  const flagDir = dirIdx >= 0 && args[dirIdx + 1] && !args[dirIdx + 1].startsWith('--')
+    ? args[dirIdx + 1]
+    : undefined;
+
+  let positionalDir: string | undefined;
+  for (let i = 1; i < args.length; i++) {
+    const arg = args[i];
+    if (arg === '--dir') {
+      i++;
+      continue;
+    }
+    if (arg === '--dry-run') continue;
+    if (arg.startsWith('--')) continue;
+    positionalDir = arg;
+    break;
+  }
+
+  return {
+    subcommand,
+    brainDir: flagDir ?? positionalDir ?? '.',
+    dryRun,
+  };
+}
+
 /**
  * Library-level backlinks check/fix. Throws on validation errors; returns a
  * structured result so Minions handlers + autopilot-cycle can surface counts.
@@ -236,16 +270,14 @@ export async function runBacklinksCore(opts: BacklinksOpts): Promise<BacklinksRe
 }
 
 export async function runBacklinks(args: string[]) {
-  const subcommand = args[0];
-  const dirIdx = args.indexOf('--dir');
-  const brainDir = dirIdx >= 0 ? args[dirIdx + 1] : '.';
-  const dryRun = args.includes('--dry-run');
+  const { subcommand, brainDir, dryRun } = parseBacklinksArgs(args);
 
   if (!subcommand || !['check', 'fix'].includes(subcommand)) {
-    console.error('Usage: gbrain check-backlinks <check|fix> [--dir <brain-dir>] [--dry-run]');
+    console.error('Usage: gbrain check-backlinks <check|fix> [dir] [--dir <brain-dir>] [--dry-run]');
     console.error('  check    Report missing back-links');
     console.error('  fix      Create missing back-links (appends to Timeline)');
-    console.error('  --dir    Brain directory (default: current directory)');
+    console.error('  dir      Brain directory (default: current directory)');
+    console.error('  --dir    Brain directory override');
     console.error('  --dry-run  Preview fixes without writing');
     process.exit(1);
   }

src/core/skillopt/orchestrator.ts

@@ -93,13 +93,7 @@ import { resolveLrSchedule } from './lr-schedule.ts';
 import { preflight, formatPreflightReport } from './preflight.ts';
 import { isRejected, loadRejectedBuffer, makeRejectedEntry, saveRejectedBuffer } from './rejected-buffer.ts';
 import { runReflect, runOneShotRewrite, describeJudges } from './reflect.ts';
-import {
-  acceptCandidate,
-  proposedPath as proposedFilePath,
-  revertAllPending,
-  skillPath,
-  writeProposed,
-} from './version-store.ts';
+import { acceptCandidate, bestPath, revertAllPending, skillPath, writeProposed } from './version-store.ts';
 import { runValidationGate, scoreSkillOnTasks } from './validate-gate.ts';
 import { ROLLOUT_SUCCESS_THRESHOLD } from './types.ts';
 import type { SkillOptOpts, EditOp, RunReceipt, BenchmarkTask } from './types.ts';
@@ -708,9 +702,9 @@ async function runOptimizationLoop(
   // to the catch's assignment values only (it can't prove the async callback ran).
   const finalOutcome = outcome as 'accepted' | 'no_improvement' | 'aborted' | 'errored';
   if (!mutateDecision.mutate && finalOutcome === 'accepted') {
-    // writeProposed() emitted both the best pointer and the stable review
-    // artifact in the accept branch. SKILL.md remains untouched.
-    proposedPath = proposedFilePath(skillsDir, skillName);
+    // best.md was written by writeProposed() in the accept branch (no-mutate
+    // path); it doubles as proposed.md for human review. SKILL.md untouched.
+    proposedPath = bestPath(skillsDir, skillName);
   } else if (mutateDecision.mutate) {
     mutatedSkillFile = finalOutcome === 'accepted';
   }

src/core/skillopt/version-store.ts

@@ -23,7 +23,6 @@
  *
  *   history.json
  *   best.md
- *   proposed.md
  *   versions/
  *     v0001_e1_s1.md
  *     v0002_e1_s2.md
@@ -53,10 +52,6 @@ export function bestPath(skillsDir: string, skillName: string): string {
   return path.join(skilloptDir(skillsDir, skillName), 'best.md');
 }
 
-export function proposedPath(skillsDir: string, skillName: string): string {
-  return path.join(skilloptDir(skillsDir, skillName), 'proposed.md');
-}
-
 export function skillPath(skillsDir: string, skillName: string): string {
   return path.join(skillsDir, skillName, 'SKILL.md');
 }
@@ -176,18 +171,17 @@ export function acceptCandidate(input: AcceptInput): AcceptResult {
 }
 
 /**
- * Write the candidate to both `best.md` and `proposed.md` WITHOUT touching
- * SKILL.md or the history ledger. `best.md` remains the optimizer's current
- * best pointer; `proposed.md` is the stable human-review artifact promised by
- * `--no-mutate`. Returns the proposal path. Each write is atomic (.tmp + rename).
+ * Write the candidate to `best.md` (which doubles as `proposed.md`) WITHOUT
+ * touching SKILL.md or the history ledger. Used by the `--no-mutate` /
+ * bundled-without-allow paths: the optimizer found a better candidate but the
+ * caller opted out of in-place mutation, so we surface it for human review.
+ * Returns the path written. Atomic (.tmp + rename).
  */
 export function writeProposed(skillsDir: string, skillName: string, candidateText: string): string {
-  const best = bestPath(skillsDir, skillName);
-  const proposed = proposedPath(skillsDir, skillName);
-  fs.mkdirSync(path.dirname(best), { recursive: true });
-  atomicWrite(best, candidateText);
-  atomicWrite(proposed, candidateText);
-  return proposed;
+  const p = bestPath(skillsDir, skillName);
+  fs.mkdirSync(path.dirname(p), { recursive: true });
+  atomicWrite(p, candidateText);
+  return p;
 }
 
 /**

test/backlinks.test.ts

@@ -4,6 +4,7 @@ import {
   extractPageTitle,
   hasBacklink,
   buildBacklinkEntry,
+  parseBacklinksArgs,
 } from '../src/commands/backlinks.ts';
 
 describe('extractEntityRefs', () => {
@@ -104,3 +105,26 @@ describe('findBacklinkGaps dedupe (v0.36.x #967 regression)', () => {
     }
   });
 });
+
+describe('parseBacklinksArgs', () => {
+  test('uses positional dir for check and fix subcommands', () => {
+    expect(parseBacklinksArgs(['check', '/tmp/brain']).brainDir).toBe('/tmp/brain');
+    expect(parseBacklinksArgs(['fix', '/tmp/brain']).brainDir).toBe('/tmp/brain');
+  });
+
+  test('defaults to cwd when no dir given', () => {
+    expect(parseBacklinksArgs(['check']).brainDir).toBe('.');
+  });
+
+  test('--dir overrides positional dir and preserves dry-run', () => {
+    const parsed = parseBacklinksArgs(['fix', '/tmp/ignored', '--dir', '/tmp/brain', '--dry-run']);
+    expect(parsed.subcommand).toBe('fix');
+    expect(parsed.brainDir).toBe('/tmp/brain');
+    expect(parsed.dryRun).toBe(true);
+  });
+
+  test('--dir missing its value falls back to positional dir', () => {
+    expect(parseBacklinksArgs(['check', '/tmp/brain', '--dir']).brainDir).toBe('/tmp/brain');
+    expect(parseBacklinksArgs(['check', '--dir', '--dry-run']).brainDir).toBe('.');
+  });
+});

test/e2e/skillopt-loop.serial.test.ts

@@ -39,7 +39,6 @@ import { runSkillOpt } from '../../src/core/skillopt/orchestrator.ts';
 import {
   bestPath,
   loadHistory,
-  proposedPath,
   skillPath,
 } from '../../src/core/skillopt/version-store.ts';
 import { loadRejectedBuffer } from '../../src/core/skillopt/rejected-buffer.ts';
@@ -742,7 +741,7 @@ describe('skillopt T3 — F11 held-out gate, ablation opts, no-DB-pollution', ()
     } finally { fixture.cleanup(); }
   });
 
-  test('--no-mutate writes proposed.md and best.md, leaves SKILL.md untouched', async () => {
+  test('--no-mutate writes proposed.md (best.md), leaves SKILL.md untouched', async () => {
     const fixture = setupFixture(SKILL_PEOPLE_ONLY, CITATIONS_BENCHMARK);
     try {
       installStub({
@@ -754,9 +753,10 @@ describe('skillopt T3 — F11 held-out gate, ablation opts, no-DB-pollution', ()
           const result = await runOnce(fixture, { noMutate: true });
           expect(result.outcome).toBe('accepted');
           expect(result.mutatedSkillFile).toBe(false);
-          expect(result.proposedPath).toBe(proposedPath(fixture.skillsDir, SKILL));
+          expect(result.proposedPath).toBeDefined();
+          // proposed.md (best.md) exists and carries the improvement.
+          expect(fs.existsSync(result.proposedPath!)).toBe(true);
           expect(fs.readFileSync(result.proposedPath!, 'utf8')).toContain('## Citations');
-          expect(fs.readFileSync(bestPath(fixture.skillsDir, SKILL), 'utf8')).toContain('## Citations');
           // SKILL.md on disk is UNCHANGED (still People-only).
           const skill = fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8');
           expect(skill).not.toContain('## Citations');

test/skillopt/version-store.test.ts

@@ -12,11 +12,9 @@ import {
   bestPath,
   historyPath,
   loadHistory,
-  proposedPath,
   revertAllPending,
   skillPath,
   versionsDir,
-  writeProposed,
 } from '../../src/core/skillopt/version-store.ts';
 
 let tmpDir: string;
@@ -81,19 +79,6 @@ describe('acceptCandidate (D8 two-phase commit)', () => {
   });
 });
 
-describe('writeProposed', () => {
-  test('writes distinct best and proposed artifacts without mutating SKILL.md (#2635)', () => {
-    const candidate = '---\nname: test\n---\nproposed body\n';
-
-    const written = writeProposed(tmpDir, SKILL, candidate);
-
-    expect(written).toBe(proposedPath(tmpDir, SKILL));
-    expect(fs.readFileSync(bestPath(tmpDir, SKILL), 'utf8')).toBe(candidate);
-    expect(fs.readFileSync(proposedPath(tmpDir, SKILL), 'utf8')).toBe(candidate);
-    expect(fs.readFileSync(skillPath(tmpDir, SKILL), 'utf8')).toContain('baseline body');
-  });
-});
-
 describe('revertAllPending (D8 crash recovery)', () => {
   test('no-op when no pending rows', () => {
     const reverted = revertAllPending(tmpDir, SKILL);