refactor(cleanup): simplify pass on R-b

The retired --profile is one list the parser drops (with its value, never
an error) instead of a valued flag in six schemas, so the removed-surfaces
check exempts one parser line, not seven; seen red on the old parser.
The empty OPTIONAL_STRING_KEYS machinery, a one-item loop and an unused
plugin-data dir go; the removed-surfaces check reads each file once; the
Phase 8 slice in executor-prose-pins asserts both anchors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

lib/parsers/arg-parser.mjs

@@ -13,37 +13,40 @@
 import { readFileSync } from 'node:fs';
 import { isMain } from '../util/is-main.mjs';
 
-// --profile: retired in cleanup R-b: the value is consumed and ignored.
+// Flags retired with a value (cleanup R-b): the flag and the word after it,
+// when that word is not a flag, are consumed and dropped, never an error.
+const RETIRED_VALUED = ['--profile'];
+
 const FLAG_SCHEMA = {
   trekbrief: {
     boolean: ['--quick', '--fg'],
-    valued: ['--profile', '--approve'],
+    valued: ['--approve'],
     aliases: {},
   },
   trekresearch: {
     boolean: ['--quick', '--local', '--external', '--fg'],
-    valued: ['--project', '--profile'],
+    valued: ['--project'],
     aliases: {},
   },
   trekplan: {
     boolean: ['--quick', '--fg'],
-    valued: ['--project', '--brief', '--export', '--decompose', '--profile'],
+    valued: ['--project', '--brief', '--export', '--decompose'],
     multi: ['--research'],
     aliases: {},
   },
   trekexecute: {
     boolean: ['--resume', '--dry-run', '--validate', '--fg'],
-    valued: ['--project', '--step', '--session', '--profile'],
+    valued: ['--project', '--step', '--session'],
     aliases: {},
   },
   trekreview: {
     boolean: ['--quick', '--fg', '--dry-run', '--validate', '--answers'],
-    valued: ['--project', '--since', '--profile'],
+    valued: ['--project', '--since'],
     aliases: {},
   },
   trekcontinue: {
     boolean: ['--help', '--cleanup', '--confirm', '--dry-run'],
-    valued: ['--profile'],
+    valued: [],
     aliases: {},
   },
   trekendsession: {
@@ -112,6 +115,11 @@ export function parseArgs(argString, command) {
       continue;
     }
 
+    if (RETIRED_VALUED.includes(tok)) {
+      if (i + 1 < tokens.length && !tokens[i + 1].startsWith('--')) i++;
+      continue;
+    }
+
     if (schema.multi && schema.multi.includes(tok)) {
       const collected = [];
       while (i + 1 < tokens.length && !tokens[i + 1].startsWith('--')) {

lib/parsers/manifest-yaml.mjs

@@ -31,15 +31,7 @@ const OPTIONAL_KEYS = [
 
 const OPTIONAL_BOOLEAN_KEYS = new Set(OPTIONAL_KEYS);
 
-// Optional string-typed manifest keys (v4.1 Step 3 — additive forward-compat).
-// Unlike OPTIONAL_BOOLEAN_KEYS, absence is NOT defaulted — the field is simply
-// missing from `parsed` so downstream consumers can distinguish "not set" from
-// "explicitly empty string".
-const OPTIONAL_STRING_KEYS = [];
-
-const OPTIONAL_STRING_KEYS_SET = new Set(OPTIONAL_STRING_KEYS);
-
-export { OPTIONAL_KEYS, OPTIONAL_STRING_KEYS };
+export { OPTIONAL_KEYS };
 
 /**
  * Extract the first fenced YAML block whose first non-blank line begins with
@@ -118,19 +110,6 @@ export function parseManifest(stepBody) {
     }
   }
 
-  // v4.1 Step 3 — string-typed optional keys. Absence == not-present (no default,
-  // unlike boolean keys above) so downstream can distinguish unset vs empty string.
-  for (const k of OPTIONAL_STRING_KEYS_SET) {
-    if (k in parsed) {
-      if (typeof parsed[k] !== 'string') {
-        errors.push(issue(
-          'MANIFEST_OPTIONAL_TYPE',
-          `${k} must be string if present (got ${typeof parsed[k]})`,
-        ));
-      }
-    }
-  }
-
   return { valid: errors.length === 0, errors, warnings, parsed };
 }
 

tests/lib/arg-parser.test.mjs

@@ -285,4 +285,8 @@ test('trekreview --answers: the parser accepts it as a boolean, and the usage li
 test('retired --profile swallows its value: --profile economy leaves positional untouched', () => {
   const r = parseArgs('--profile economy .claude/projects/x', 'trekcontinue');
   assert.deepEqual(r.positional, ['.claude/projects/x'], `the retired flag's value is no positional: ${JSON.stringify(r)}`);
+  assert.deepEqual([r.flags, r.unknown, r.errors], [{}, [], []], `the retired flag is dropped, not reported: ${JSON.stringify(r)}`);
+  const bare = parseArgs('--project x --since HEAD~1 --profile', 'trekreview');
+  assert.deepEqual(bare.errors, [], `a bare retired flag is no error: ${JSON.stringify(bare)}`);
+  assert.equal(parseArgs('--profile --project x', 'trekreview').flags['--project'], 'x', 'the retired flag never takes a flag as its value');
 });

tests/lib/executor-prose-pins.test.mjs

@@ -223,7 +223,10 @@ test('P4: the run stops only on the closed list of four, and the final report sh
   assert.match(list, /closed/i);
   // Phase 8's report template is a fence that holds its own "## " heading, so
   // the phase is read up to Hard rules, not up to the next heading.
-  const report = text.slice(text.indexOf('## Phase 8 — Final report'), text.indexOf('## Hard rules'));
+  const start = text.indexOf('## Phase 8 — Final report');
+  const end = text.indexOf('## Hard rules');
+  assert.ok(start >= 0 && end > start, 'Phase 8 and Hard rules must both exist, in that order');
+  const report = text.slice(start, end);
   assert.match(report, /### Stops/);
   assert.match(report, /### Decisions taken/);
 });

tests/lib/manifest-schema-extensions.test.mjs

@@ -8,7 +8,7 @@
 
 import { test } from 'node:test';
 import { strict as assert } from 'node:assert';
-import { parseManifest, OPTIONAL_KEYS, OPTIONAL_STRING_KEYS } from '../../lib/parsers/manifest-yaml.mjs';
+import { parseManifest, OPTIONAL_KEYS } from '../../lib/parsers/manifest-yaml.mjs';
 
 const BASE = `### Step 1: Cover
 - Manifest:
@@ -91,8 +91,3 @@ test('extension does NOT break REQUIRED_KEYS contract', () => {
   }
 });
 
-// v4.1 Step 3 — OPTIONAL_STRING_KEYS dispatch; its one key went in cleanup R-b.
-
-test('OPTIONAL_STRING_KEYS exports no key', () => {
-  assert.deepEqual([...OPTIONAL_STRING_KEYS], [], 'OPTIONAL_STRING_KEYS export drift — pin contract');
-});

tests/lib/removed-surfaces.test.mjs

@@ -26,7 +26,7 @@ const SELF = 'tests/lib/removed-surfaces.test.mjs';
 
 const flag = name => new RegExp(`(?<![\\w-])${name}(?![\\w-])`);
 
-export const GROUPS = {
+const GROUPS = {
   R1: [
     'lib/stats/', 'lib/exporters/', 'otel-export', 'post-bash-stats', 'stats-append', 'event-emit',
     'yardstick.mjs', 'scripts/yardstick', 'end-state-gate', 'end-state-registry', 'comparison-gate',
@@ -52,15 +52,11 @@ const EXEMPT = new Map([
   ['docs/proevesett.md:139', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
   ['docs/proevesett.md:143', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
   ['docs/proevesett.md:146', 'dated v1/v2 measurement record of the fixed test set, kept unedited'],
-  ['lib/parsers/arg-parser.mjs:16', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:20', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:25', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:30', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:36', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:41', 'retired flag swallows its value'],
-  ['lib/parsers/arg-parser.mjs:46', 'retired flag swallows its value'],
+  ['lib/parsers/arg-parser.mjs:18', 'retired flag swallows its value'],
   ['tests/lib/arg-parser.test.mjs:285', 'retired flag swallows its value'],
   ['tests/lib/arg-parser.test.mjs:286', 'retired flag swallows its value'],
+  ['tests/lib/arg-parser.test.mjs:289', 'retired flag swallows its value'],
+  ['tests/lib/arg-parser.test.mjs:291', 'retired flag swallows its value'],
   ['tests/validators/brief-validator.test.mjs:200', 'older brief stays valid (decision 191)'],
   ['tests/validators/brief-validator.test.mjs:203', 'older brief stays valid (decision 191)'],
   ['tests/validators/brief-validator.test.mjs:205', 'older brief stays valid (decision 191)'],
@@ -68,20 +64,29 @@ const EXEMPT = new Map([
 
 const matches = (term, line) => (typeof term === 'string' ? line.includes(term) : term.test(line));
 
+// Each file is read and split once for all groups; a binary file has no lines.
+const lineCache = new Map();
+function linesOf(root, rel) {
+  const path = join(root, rel);
+  if (!lineCache.has(path)) {
+    const buf = readFileSync(path);
+    lineCache.set(path, buf.includes(0) ? [] : buf.toString('utf-8').split('\n'));
+  }
+  return lineCache.get(path);
+}
+
 // "path:line" for every line of the given files that names a term of the group.
-export function scan(root, files, terms) {
+function scan(root, files, terms) {
   const hits = [];
   for (const rel of files) {
-    const buf = readFileSync(join(root, rel));
-    if (buf.includes(0)) continue; // binary
-    buf.toString('utf-8').split('\n').forEach((line, i) => {
+    linesOf(root, rel).forEach((line, i) => {
       if (terms.some(t => matches(t, line))) hits.push(`${rel}:${i + 1}`);
     });
   }
   return hits;
 }
 
-export function inScope(rel) {
+function inScope(rel) {
   return rel !== 'CHANGELOG.md' && rel !== SELF && !/\.local\./.test(rel);
 }
 
@@ -130,9 +135,10 @@ for (const group of Object.keys(GROUPS)) {
 
 test('every EXEMPT entry still names a removed surface', () => {
   const terms = Object.values(GROUPS).flat();
+  const all = new Set(tracked());
   for (const key of EXEMPT.keys()) {
     const rel = key.replace(/:\d+$/, '');
-    assert.ok(tracked().includes(rel), `EXEMPT names untracked ${rel}`);
+    assert.ok(all.has(rel), `EXEMPT names untracked ${rel}`);
     assert.ok(scan(ROOT, [rel], terms).includes(key), `EXEMPT entry "${key}" no longer hits; remove it`);
   }
 });

tests/lib/review-630.test.mjs

@@ -908,11 +908,10 @@ test('review 630 r8 / criteria-runner.mjs:890: no CLI compares process.argv[1] i
   const link = join(base, 'voyage link');
   try {
     symlinkSync(ROOT, link);
-    for (const cli of ['lib/verification/criteria-runner.mjs']) {
-      const r = spawnSync(process.execPath, [join(link, cli), '--no-such-flag'], { encoding: 'utf8', input: '', timeout: 30_000 });
-      assert.equal(r.status, 2, `${cli} through "${link}": ${r.stdout}${r.stderr}`);
-      assert.match(r.stderr, /unknown argument --no-such-flag/);
-    }
+    const cli = 'lib/verification/criteria-runner.mjs';
+    const r = spawnSync(process.execPath, [join(link, cli), '--no-such-flag'], { encoding: 'utf8', input: '', timeout: 30_000 });
+    assert.equal(r.status, 2, `${cli} through "${link}": ${r.stdout}${r.stderr}`);
+    assert.match(r.stderr, /unknown argument --no-such-flag/);
   } finally { fs.rmSync(link, { force: true }); rmSync(base, { recursive: true, force: true }); }
 });
 

tests/validators/brief-validator.test.mjs

@@ -160,8 +160,8 @@ test('validateBrief — wrong-type error message includes accepted set', () => {
 // --- v5.5 — framing enforcement + obligatory TL;DR (gated at brief_version ≥ 2.2) ---
 // Operator decision (S6, option A1): framing + TL;DR are hard BLOCKERs for briefs
 // declaring brief_version ≥ 2.2; existing 2.0/2.1 briefs stay valid (forward-compat).
-// The framing ENUM check fires on any
-// version when the field is present but malformed.
+// The framing ENUM check fires on any version when the field is present but
+// malformed.
 
 const GOOD_BRIEF_22 = `---
 type: trekbrief

tests/validators/intent-approval.test.mjs

@@ -364,18 +364,16 @@ test('M2: /trekbrief --approve <project-dir> still resolves to <dir>/brief.md an
 
 test('M2: the Phase 4h stamp line stamps the resolved brief file, whatever its name', () => {
   const dir = tmpDir();
-  const data = tmpDir();
   try {
     const p = join(dir, 'x-brief.md');
     writeFileSync(p, BRIEF);
     const line = commandLine('commands/trekbrief.md', '--stamp');
     assert.ok(line.includes('{BRIEF_PATH}'), 'the stamp must take the resolved brief path, not {PROJECT_DIR}/brief.md');
-    const r = run(line, { '{BRIEF_PATH}': p }, { CLAUDE_PLUGIN_DATA: data });
+    const r = run(line, { '{BRIEF_PATH}': p });
     assert.equal(r.status, 0, r.stdout + r.stderr);
     assert.equal(gate(p).status, 0);
   } finally {
     rmSync(dir, { recursive: true, force: true });
-    rmSync(data, { recursive: true, force: true });
   }
 });