fix(agent-model): pre-landing review fixes

- a broken preferences.yaml no longer blocks --request or config.yaml
- CRLF, a byte-order mark and a trailing comment still read the value
- never echo raw control or bidi bytes from an unvalidated source
- cap the bytes actually read, not only the stat size
- family names and host are case-insensitive in gstack-config too
- list passes the platform through to the agent block
- a wrapper inside the checkout names no preferences file
- docs: --check never writes the skill folder

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

bin/gstack-capy-session

@@ -92,11 +92,16 @@ report_behind() {
 }
 unset GSTACK_CAPY_BEHIND GSTACK_CAPY_BEHIND_TO GSTACK_CAPY_BEHIND_FIX GSTACK_CAPY_BEHIND_BLOCKED
 # The owner's preferences.yaml sits beside the Drive skill's SKILL.md and is
-# read where it lies (never copied), so --check stays read-only and a newer
-# file is seen by the next command. The installer writes the path into the
-# launch file; gstack-models resolve --role agent reads it.
+# read where it lies (never copied), so --check never writes the skill folder
+# and a newer file is seen by the next command. The installer writes the path
+# into the launch file; gstack-models resolve --role agent reads it. A wrapper
+# inside this checkout is no Drive skill, so it names no preferences file.
 if [ -n "${GSTACK_CAPY_WRAPPER_PATH:-}" ]; then
-  export GSTACK_PREFERENCES_FILE="$(dirname "$(dirname "$GSTACK_CAPY_WRAPPER_PATH")")/preferences.yaml"
+  _prefs_dir="$(dirname "$(dirname "$GSTACK_CAPY_WRAPPER_PATH")")"
+  case "$_prefs_dir/" in
+    "$ROOT"/*) ;;
+    *) export GSTACK_PREFERENCES_FILE="$_prefs_dir/preferences.yaml" ;;
+  esac
 fi
 RERUN="bash ${GSTACK_CAPY_WRAPPER_PATH:-<skill>/scripts/install.sh}${SKILL:+ --for $SKILL}"
 if [ "$CHECK" -eq 1 ]; then

bin/gstack-config

@@ -843,7 +843,7 @@ $_gstack_legacy_root"
     if [ "$KEY" = "agent_model" ]; then
       VALUE="${VALUE#"${VALUE%%[![:space:]]*}"}"
       VALUE="${VALUE%"${VALUE##*[![:space:]]}"}"
-      case "$VALUE" in
+      case "$(printf '%s' "$VALUE" | LC_ALL=C tr 'A-Z' 'a-z')" in
         host|opus|fable|sonnet|haiku|astra|sol|luna|terra) ;;
         *) printf '%s' "$VALUE" | LC_ALL=C grep -qE '^[a-z][a-z0-9-]{0,31}/[A-Za-z0-9][A-Za-z0-9._:@+-]{0,199}$' || {
              echo "Error: agent_model '$VALUE' not recognized. Use host, a family (opus, fable, sonnet, haiku, astra, sol, luna, terra) or <provider>/<model> such as anthropic/claude-opus-5-5. Existing value left unchanged. fix: gstack-config unset agent_model returns to host (https://github.com/garrytan/gstack/blob/main/docs/model-policy.md#your-model-preference)" >&2

docs/model-policy.md

@@ -157,7 +157,7 @@ over. Keep the preference in `preferences.yaml` next to the gstack skill's
 SKILL.md in your Drive: one `agent_model: <value>` line, with `#` comments
 allowed (any other key is ignored with a warning). The session start puts the
 file's path in the launch file as `GSTACK_PREFERENCES_FILE` and gstack reads it
-where it is: nothing is copied, and `--check` writes nothing. Agents write that
+where it is: nothing is copied, and `--check` never writes the skill folder. Agents write that
 file only when you ask them to remember a preference. To change or clear it for
 good, edit or delete that line; `gstack-config unset agent_model` clears this
 machine only. Copying a newer `capy/skill/gstack/` over the skill folder leaves

lib/agent-model.ts

@@ -62,8 +62,12 @@ export class AgentModelError extends Error {
   }
 }
 
+function printable(value: string): string {
+  return value.replace(/[\x00-\x1f\x7f-\x9f\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]/g, ' ');
+}
+
 function sanitize(value: string): string {
-  return value.replace(/[\x00-\x1f\x7f]/g, ' ').slice(0, 200);
+  return printable(value).slice(0, 200);
 }
 
 type Parsed = { kind: 'host' } | { kind: 'model'; provider: string; model: string; family: string | null };
@@ -71,7 +75,7 @@ type Parsed = { kind: 'host' } | { kind: 'model'; provider: string; model: strin
 /** host | a family name | <provider>/<model>. Syntax only; bin/gstack-config holds the same grammar. */
 export function parseAgentModel(raw: string): Parsed | null {
   const value = raw.trim();
-  if (value === 'host') return { kind: 'host' };
+  if (value.toLowerCase() === 'host') return { kind: 'host' };
   const family = MODEL_FAMILIES.find(f => f.family === value.toLowerCase());
   if (family) return { kind: 'model', provider: family.provider, model: family.model, family: family.family };
   const slash = value.indexOf('/');
@@ -96,22 +100,23 @@ export function readPreferencesFile(file: string): { value: string | null; warni
   try {
     const stat = fs.statSync(file);
     if (!stat.isFile()) return { value: null, warnings: [`${file} is not a file; ignored`] };
-    if (stat.size > PREFERENCES_FILE_MAX_BYTES) return { value: null, warnings: [`${file} is over ${PREFERENCES_FILE_MAX_BYTES} bytes; ignored`] };
-    text = fs.readFileSync(file, 'utf-8');
+    const bytes = stat.size > PREFERENCES_FILE_MAX_BYTES ? null : fs.readFileSync(file);
+    if (!bytes || bytes.length > PREFERENCES_FILE_MAX_BYTES) return { value: null, warnings: [`${file} is over ${PREFERENCES_FILE_MAX_BYTES} bytes; ignored`] };
+    text = bytes.toString('utf-8').replace(/^\uFEFF/, '');
   } catch (error) {
     const code = (error as NodeJS.ErrnoException).code;
     return { value: null, warnings: code === 'ENOENT' ? [] : [`${file} is unreadable (${code ?? 'unknown error'}); ignored`] };
   }
   const warnings: string[] = [];
   let value: string | null = null;
-  text.split('\n').forEach((line, i) => {
+  text.split(/\r?\n/).forEach((line, i) => {
     const trimmed = line.trim();
     if (trimmed === '' || trimmed.startsWith('#')) return;
     const m = /^([a-z][a-z0-9_]*):(.*)$/.exec(line);
     if (!m) { warnings.push(`${file}:${i + 1} is not a 'key: value' line; ignored`); return; }
     if (m[1] !== AGENT_MODEL_KEY) { warnings.push(`${file}:${i + 1} sets '${sanitize(m[1])}', which gstack does not read from preferences; ignored`); return; }
     if (value !== null) throw new AgentModelError(`${file} sets agent_model twice`, 'keep one agent_model line', [`edit ${file}`]);
-    value = m[2].trim().replace(/^(["'])(.*)\1$/, '$2');
+    value = m[2].replace(/\s#.*$/, '').trim().replace(/^(["'])(.*)\1$/, '$2');
   });
   return { value: value === '' ? null : value, warnings };
 }
@@ -121,7 +126,13 @@ export function resolveAgentModel(opts: { env?: StateRootEnv; platform?: NodeJS.
   const reading = readConfigKeyWithRoot('agent_model', env, opts.platform ?? process.platform);
   const configPath = reading.root ? path.join(reading.root, 'config.yaml') : null;
   const preferencesPath = env.GSTACK_PREFERENCES_FILE?.trim() || null;
-  const prefs = preferencesPath ? readPreferencesFile(preferencesPath) : { value: null, warnings: [] };
+  let prefs: { value: string | null; warnings: string[] } = { value: null, warnings: [] };
+  try {
+    if (preferencesPath) prefs = readPreferencesFile(preferencesPath);
+  } catch (error) {
+    if (!(error instanceof AgentModelError) || (opts.request === undefined && reading.value === null)) throw error;
+    prefs = { value: null, warnings: [`${error.problem}; ignored because a higher-priority source is set`] };
+  }
   const candidates: Array<{ kind: AgentModelSourceKind; value: string | null; path: string | null; repair: string[] }> = [
     { kind: 'request', value: opts.request ?? null, path: null, repair: ['pass --request host, a family or <provider>/<model>'] },
     { kind: 'config', value: reading.value, path: configPath, repair: ['gstack-config set agent_model <host|family|provider/model>', 'gstack-config unset agent_model'] },
@@ -150,8 +161,8 @@ export function renderAgentModel(s: AgentModelSelection): string[] {
       : `from agent_model in ${s.source.path}, ${s.source.kind === 'config' ? 'this machine' : 'the preferences file'}`;
   const lines = [s.status === 'host'
     ? `agent: host - keep the running agent's own model and omit the model field (${from})`
-    : `agent: ${s.id}${s.family ? ` (${s.family})` : ''} - ${from}`];
-  lines.push(`  config.yaml agent_model: ${s.sources.config ?? 'unset'}; preferences agent_model: ${s.sources.preferences ?? 'unset'}${s.sources.preferencesPath ? ` (${s.sources.preferencesPath})` : ' (GSTACK_PREFERENCES_FILE unset)'}`);
+    : `agent: ${s.id}${s.family ? ` (${s.family})` : ''} - ${from}`].map(printable);
+  lines.push(printable(`  config.yaml agent_model: ${s.sources.config ?? 'unset'}; preferences agent_model: ${s.sources.preferences ?? 'unset'}${s.sources.preferencesPath ? ` (${s.sources.preferencesPath})` : ' (GSTACK_PREFERENCES_FILE unset)'}`));
   lines.push(`  exempt: ${s.exemptions.join('; ')}`);
   lines.push('  change it on this machine: gstack-config set agent_model <host|family|provider/model>; keep it across Capy machines: agent_model in the skill folder\'s preferences.yaml');
   for (const w of s.warnings) lines.push(`  warning: ${sanitize(w)}`);

lib/model-policy-cli.ts

@@ -191,7 +191,7 @@ function renderSelections(role: ModelRole, config: ModelPolicyConfig, selections
   return lines;
 }
 
-function renderList(config: ModelPolicyConfig, selections: ModelSelection[], env: StateRootEnv): string {
+function renderList(config: ModelPolicyConfig, selections: ModelSelection[], env: StateRootEnv, platform: NodeJS.Platform): string {
   const effort = codexEffortSetting(env);
   const commands = modelPolicyCommands();
   const configCommand = `'${CONFIG_BIN.replaceAll('\\', '/').replaceAll("'", "'\\''")}'`;
@@ -204,7 +204,7 @@ function renderList(config: ModelPolicyConfig, selections: ModelSelection[], env
     '',
     `codex effort: ${effort.value ?? 'each gate\'s default'} (${effort.source === 'env' ? 'GSTACK_CODEX_EFFORT' : effort.source === 'config' ? `codex_effort in ${config.path}` : 'default; set codex_effort'})${effort.valid ? '' : ` - invalid; Codex calls stop before sending. Use ${CODEX_EFFORT_LEVELS.join(', ')}`}; /spec keeps medium`,
     '',
-    ...agentLines(env),
+    ...agentLines(env, platform),
     '',
     `catalog v${MODEL_CATALOG_VERSION} (sha256 ${modelCatalogSha256().slice(0, 12)})`,
   ];
@@ -260,8 +260,8 @@ function renderAgentError(error: ReturnType<AgentModelError['toJSON']>): string[
 }
 
 /** The agent block of `list`: a bad agent_model is reported here and never fails the other roles. */
-function agentLines(env: StateRootEnv): string[] {
-  const agent = agentOrError(env);
+function agentLines(env: StateRootEnv, platform: NodeJS.Platform): string[] {
+  const agent = agentOrError(env, platform);
   return 'error' in agent ? renderAgentError(agent.error) : renderAgentModel(agent);
 }
 
@@ -334,8 +334,8 @@ export function modelsMain(args: string[], env: StateRootEnv = process.env, plat
   const code = errors.length ? 1 : 0;
   if (json) {
     const doc = document(command, config, selections, errors, env);
-    return { code, stdout: `${JSON.stringify(command === 'list' ? { ...doc, agent: agentOrError(env) } : doc, null, 2)}\n`, stderr: '' };
+    return { code, stdout: `${JSON.stringify(command === 'list' ? { ...doc, agent: agentOrError(env, platform) } : doc, null, 2)}\n`, stderr: '' };
   }
-  const stdout = command === 'list' ? renderList(config, selections, env) : role === 'eval-arms' ? renderEvalArms(config, selections) : renderResolve(role as ModelRole, config, selections);
+  const stdout = command === 'list' ? renderList(config, selections, env, platform) : role === 'eval-arms' ? renderEvalArms(config, selections) : renderResolve(role as ModelRole, config, selections);
   return { code, stdout, stderr: errors.map(renderError).join('') };
 }

test/agent-model.test.ts

@@ -12,7 +12,7 @@ import { spawnSync } from 'child_process';
 import * as fs from 'fs';
 import * as os from 'os';
 import * as path from 'path';
-import { AgentModelError, MODEL_FAMILIES, parseAgentModel, readPreferencesFile, resolveAgentModel } from '../lib/agent-model';
+import { AgentModelError, MODEL_FAMILIES, parseAgentModel, readPreferencesFile, renderAgentModel, resolveAgentModel } from '../lib/agent-model';
 import { MODEL_CATALOG } from '../lib/model-catalog';
 import { modelsMain } from '../lib/model-policy-cli';
 
@@ -37,6 +37,7 @@ describe('value grammar', () => {
     expect(parseAgentModel('host')).toEqual({ kind: 'host' });
     expect(parseAgentModel(' opus ')).toEqual({ kind: 'model', provider: 'anthropic', model: 'claude-opus-5-5', family: 'opus' });
     expect(parseAgentModel('OPUS')).toMatchObject({ family: 'opus' });
+    expect(parseAgentModel('Host')).toEqual({ kind: 'host' });
     expect(parseAgentModel('openai/gpt-6.1-sol')).toEqual({ kind: 'model', provider: 'openai', model: 'gpt-6.1-sol', family: null });
     for (const bad of ['', 'opux', 'claude-opus-5-5', 'Anthropic/x', 'anthropic/', '/x', 'anthropic/a b', 'anthropic/x\u001b[31m']) expect(parseAgentModel(bad)).toBeNull();
   });
@@ -50,7 +51,7 @@ describe('value grammar', () => {
 
   test('bin/gstack-config accepts and refuses exactly what the resolver does', () => {
     const e = env(tmp());
-    for (const value of ['host', 'opus', 'sonnet', 'terra', 'anthropic/claude-opus-5-5', 'openai/gpt-6.1-sol', 'opux', 'claude-opus-5-5', 'Anthropic/x', 'anthropic/a b', 'anthropic/']) {
+    for (const value of ['host', 'Host', 'opus', 'OPUS', 'Sol', 'sonnet', 'terra', 'anthropic/claude-opus-5-5', 'openai/gpt-6.1-sol', 'opux', 'claude-opus-5-5', 'Anthropic/x', 'anthropic/a b', 'anthropic/']) {
       const r = config(e, ['set', 'agent_model', value]);
       expect(r.status === 0, value + r.stderr).toBe(parseAgentModel(value) !== null);
     }
@@ -88,6 +89,22 @@ describe('precedence and sources', () => {
     expect(resolveAgentModel({ env: e })).toMatchObject({ id: 'anthropic/claude-opus-5-5', source: { kind: 'preferences' } });
   });
 
+  test('a broken preferences file never blocks a request or this machine\'s config.yaml, and its raw value is never echoed', () => {
+    const base = tmp();
+    const prefs = path.join(base, 'preferences.yaml');
+    fs.writeFileSync(prefs, 'agent_model: opus\nagent_model: sol\n');
+    const e = env(base, { GSTACK_PREFERENCES_FILE: prefs });
+    expect(() => resolveAgentModel({ env: e })).toThrow(/sets agent_model twice/);
+    const requested = resolveAgentModel({ env: e, request: 'sonnet' });
+    expect(requested).toMatchObject({ family: 'sonnet', source: { kind: 'request' } });
+    expect(requested.warnings.join('\n')).toContain('sets agent_model twice');
+    setConfig(e, 'agent_model: opus\n');
+    expect(resolveAgentModel({ env: e })).toMatchObject({ family: 'opus', source: { kind: 'config' } });
+    fs.writeFileSync(prefs, 'agent_model: x\u001b]0;title\u0007\u009b31m\u202e\n');
+    const shown = renderAgentModel(resolveAgentModel({ env: e })).join('\n');
+    expect(shown).not.toMatch(/[\u0000-\u0008\u000b-\u001f\u007f-\u009f\u202a-\u202e\u2066-\u2069]/);
+  });
+
   test('an invalid value at the winning source is an error naming the source and the fix, never a silent fallback', () => {
     const e = env(tmp());
     setConfig(e, 'agent_model: opux\n');
@@ -151,6 +168,12 @@ describe('the preferences file', () => {
     expect(() => readPreferencesFile(f)).toThrow(/sets agent_model twice/);
   });
 
+  test('CRLF line endings, a byte-order mark and a trailing comment still read the value', () => {
+    const f = path.join(tmp(), 'preferences.yaml');
+    fs.writeFileSync(f, '\ufeff# owner preferences\r\nagent_model: opus   # my pick\r\n');
+    expect(readPreferencesFile(f)).toEqual({ value: 'opus', warnings: [] });
+  });
+
   test('a missing file is silent; an oversized file or a directory is ignored with a warning', () => {
     const base = tmp();
     expect(readPreferencesFile(path.join(base, 'absent.yaml'))).toEqual({ value: null, warnings: [] });
@@ -183,5 +206,7 @@ describe('Capy wiring', () => {
     expect(r.stdout).toContain('PREFS=' + path.join(skill, 'preferences.yaml'));
     expect(fs.readdirSync(skill).sort()).toEqual(before);
     expect(run({}).stdout).toContain('PREFS=unset');
+    const inCheckout = run({ GSTACK_CAPY_WRAPPER_REV: '3', GSTACK_CAPY_WRAPPER_PATH: path.join(root, 'capy', 'skill', 'gstack', 'scripts', 'install.sh') });
+    expect(inCheckout.stdout).toContain('PREFS=unset');
   });
 });