gosalyn: C2-20b persona import follow-up, keys from config.yaml print as JSON strings, the dry run applies C2-08's fence and refuses, privateDir refuses an EEXIST race by name, a test and a killed mutant for each (row 247, #1749, webby-0073 N1 N2 N4, sage-1969, sage-1971)

agents/gosalyn/work/queue-247/BUILD.md

@@ -0,0 +1,87 @@
+# Row 247 build notes, C2-20b persona import follow-up (#1749)
+
+Brief: docs/plans/2026-10-20_case2-rows.md "C2-20b persona import follow-up: printed keys, the dry run
+fence check and the mkdir race", landed in 335bb3e30. These are Webby's row 221 notes N1, N2 and N4
+(webby-0073, REVIEW-r1 a0a00630e on webby/reviews). N3 is in DEFERRED (sage-1941).
+
+## Base
+
+Branch gosalyn/c2-20b from 335bb3e30, which is row 221's landing 300766450 plus the brief.
+
+## What changed
+
+- N1, `scripts/case2-persona-import.mjs`: the per-channel prompt line prints the platform key, with its
+  fixed `platforms.` or `gateway.platforms.` prefix, and the channel key through `JSON.stringify`. A key
+  holding a newline prints as `\n` inside one quoted line. The profile name on that line is not config
+  text: `pairing` already holds it to `PROFILE_ID`.
+- N2: the dry run calls `personaTextFault` on each persona after its count line. A fault prints as
+  `profile P: persona <reason>, so C2-08's fence would refuse it`, where the reason is the fixed string
+  `personaTextFault` returns, never persona text or a line number. With any fault the run refuses, exit
+  1, and writes nothing, as stage would. The header comment no longer says a fault is counted, not
+  refused, and now says a SOUL.md over four times the fence in bytes refuses before any count, which is
+  what `soul()` already did (Webby's second point under N2).
+- N4: `privateDir` catches an EEXIST from its mkdir and refuses with `<path> was made by something else
+  between the check and mkdir`. Any other mkdir error is thrown as before, since the brief names only
+  EEXIST.
+- Tests, `packages/cli/tests/case2-persona-import.test.mjs`:
+  - The per-channel prompt test expects the quoted keys.
+  - New: a chet config.yaml with a newline in the platform key and in the channel key prints one quoted
+    line, and no output line starts with the forged text.
+  - The dry run heading test now expects the fence fault line, exit 1 and the refusal, still with
+    nothing written. Its persona has two heading lines, which the fence refuses.
+  - New: a persona of 8001 two-byte characters, 16002 bytes and under the 32000 byte cap, makes the dry
+    run print the over 8000 characters fault, write nothing and exit 1. At exactly 8000 it exits 0.
+  - New: the mkdir race refuses by name at stage, for the staging directory, and at install, for
+    `personas/<business>`. Each refused directory is left empty.
+- New fixture `packages/cli/tests/fixtures/case2-persona-import/mkdir-race.mjs`, an `--import` preload
+  like `trace.mjs`. It makes the directory named by `$PERSONA_MKDIR_RACE` just before the script's own
+  `mkdirSync` of that path, so the script meets a real EEXIST from the kernel, not a thrown stub.
+
+## Known limit of N1
+
+`JSON.stringify` escapes C0 controls, `"` and `\`, but leaves DEL, C1, U+2028, U+2029 and the bidi
+controls raw. None of those ends a line on a terminal, so the forged line N1 names can't happen. A bidi
+control could still reorder how the rest of that one line displays. The brief asks for `JSON.stringify`,
+and Sage kept it in sage-1964 because the keys come from Jason's own Hermes config. Note for Webby: row
+231's `quote()` in packages/library covers the wider set, and this row doesn't import it. If you want it,
+ask for it in review.
+
+## Mutants
+
+One per fix, plus a second for N1 so each of its two keys has its own. Each replaces one exact string
+in the script, runs the test file, then restores the script from a copy and checks its sha256.
+
+| Mutant | Fix | Change |
+|---|---|---|
+| M1 | N1 | the channel key prints raw |
+| M2 | N1 | the platform key prints raw |
+| M3 | N2 | the dry run's refusal after a fault is skipped |
+| M4 | N4 | privateDir's EEXIST branch never matches, so EEXIST is thrown raw |
+
+## Outside readers
+
+`git grep` over packages/*/tests/* and scripts/test-*.sh finds `case2-persona-import` only in its own
+test file and in two files of its fixture tree, README.md and trace.mjs, which name the test file in a
+comment. No test pins a hash of the script.
+
+## Tests
+
+Two slot runs, `busy.sh && run.sh` in /mnt/storage/scratch/gosalyn-r247, on the test file alone.
+
+Run 1, grant sage-1969, 11:47:45Z to 11:48:00Z, was red: 14 tests, 12 pass, 2 fail. The fault was in the
+script. Adding `JSON.stringify` to the per-channel line dropped the space after the comma, so it printed
+`"-1001",29 bytes`. The two per-channel tests caught it. run.sh also went on to the mutants with a red
+base, so their counts meant nothing. Sage approved both fixes in sage-1971: the space goes back, and run.sh
+now exits 3 before the mutants unless the base is rc=0 with fail 0.
+
+Run 2, grant sage-1971, 11:49:04Z to 11:49:22Z, was green: 14 tests, 14 pass, 0 fail, rc=0.
+
+| Mutant | Line | rc | Failed | Killed by |
+|---|---|---|---|---|
+| M1 | 237 | 1 | 2 | the per-channel prompt test and the newline key test |
+| M2 | 237 | 1 | 2 | the same two |
+| M3 | 238 | 1 | 2 | the dry run fence test and the dry run heading test |
+| M4 | 192 | 1 | 1 | the mkdir race test |
+
+After the run the script's sha256 was 7031d2c0dc9bd22bc748b364a35b461fc01cb98b795032a2e0ec436f3188d4e4, the
+same as the copy taken before it. The /proc scan found no process in the worktree or the scratch dir.

packages/cli/tests/case2-persona-import.test.mjs

@@ -18,6 +18,7 @@ const REPO = fileURLToPath(new URL("../../../", import.meta.url));
 const SCRIPT = join(REPO, "scripts", "case2-persona-import.mjs");
 const FIXTURE = join(REPO, "packages", "cli", "tests", "fixtures", "case2-persona-import");
 const TRACE = join(FIXTURE, "trace.mjs");
+const MKDIR_RACE = join(FIXTURE, "mkdir-race.mjs");
 const PAIRS = ["--pair", "default=Bean", "--pair", "chet=Chet"];
 const sha256 = (data) => createHash("sha256").update(data).digest("hex");
 const mode = (path) => statSync(path).mode & 0o777;
@@ -36,15 +37,15 @@ function scratch(t) {
   return { root, hermes, configDir, staging: join(root, "staging"), trace: join(root, "trace.log") };
 }
 
-function run(argv, { trace } = {}) {
-  const pre = trace ? ["--import", TRACE] : [];
-  const env = { ...process.env, ...(trace ? { PERSONA_TRACE: trace } : {}) };
+function run(argv, { trace, race } = {}) {
+  const pre = [...(trace ? ["--import", TRACE] : []), ...(race ? ["--import", MKDIR_RACE] : [])];
+  const env = { ...process.env, ...(trace ? { PERSONA_TRACE: trace } : {}), ...(race ? { PERSONA_MKDIR_RACE: race } : {}) };
   const r = spawnSync(process.execPath, [...pre, SCRIPT, ...argv], { encoding: "utf8", env, timeout: 30_000 });
   return { status: r.status, out: r.stdout, err: r.stderr, all: r.stdout + r.stderr };
 }
 
 const stage = (s, extra = [], opts) => run(["stage", "--hermes-root", s.hermes, ...PAIRS, "--staging", s.staging, ...extra], opts);
-const install = (s, business = "case2") => run(["install", "--staging", s.staging, "--config-dir", s.configDir, "--business", business]);
+const install = (s, business = "case2", opts) => run(["install", "--staging", s.staging, "--config-dir", s.configDir, "--business", business], opts);
 
 // Every fixture marker and every line of fixture text the script reads.
 function assertNoText(s, output) {
@@ -102,18 +103,38 @@ test("stage lists the per-channel prompts in all three places by name and byte c
   assert.equal(r.status, 0, r.all);
   const lines = r.out.split("\n").filter((l) => l.includes(": per-channel prompt "));
   assert.deepEqual(lines, [
-    `profile default: per-channel prompt discord 111111111111111111, ${Buffer.byteLength("marker-prompt-top-5d02 Help with the novel.")} bytes`,
-    `profile default: per-channel prompt discord 222222222222222222, ${Buffer.byteLength("Ünïcode prompt")} bytes`,
+    `profile default: per-channel prompt "discord" "111111111111111111", ${Buffer.byteLength("marker-prompt-top-5d02 Help with the novel.")} bytes`,
+    `profile default: per-channel prompt "discord" "222222222222222222", ${Buffer.byteLength("Ünïcode prompt")} bytes`,
     // Unquoted in the fixture, and past 2 ** 53: the id prints exactly.
-    "profile default: per-channel prompt discord 1333333333333333333, 5 bytes",
-    `profile default: per-channel prompt platforms.slack C0FIXTURE, ${Buffer.byteLength("marker-prompt-platforms-8e3a")} bytes`,
-    "profile default: per-channel prompt gateway.platforms.telegram -1001, 29 bytes",
-    "profile default: per-channel prompt gateway.platforms.telegram -1002, not text",
+    'profile default: per-channel prompt "discord" "1333333333333333333", 5 bytes',
+    `profile default: per-channel prompt "platforms.slack" "C0FIXTURE", ${Buffer.byteLength("marker-prompt-platforms-8e3a")} bytes`,
+    'profile default: per-channel prompt "gateway.platforms.telegram" "-1001", 29 bytes',
+    'profile default: per-channel prompt "gateway.platforms.telegram" "-1002", not text',
   ]);
   assert.ok(r.out.includes("profile chet: no per-channel prompts"), r.out);
   assertNoText(s, r.all);
 });
 
+// C2-20b N1 (webby-0073): a platform or channel key holding a newline prints as one quoted line, so
+// it can't forge a line of the script's own output.
+test("a config key with a newline prints as one quoted line", (t) => {
+  const s = scratch(t);
+  writeFileSync(join(s.hermes, "profiles", "chet", "config.yaml"), [
+    '"discord\\nstaged: /forged/bean.md":',
+    "  channel_prompts:",
+    '    "1\\npairing: profile chet is Bean": marker-prompt-forge-4c1e',
+    "",
+  ].join("\n"));
+  const r = stage(s);
+  assert.equal(r.status, 0, r.all);
+  const lines = r.out.split("\n");
+  assert.deepEqual(lines.filter((l) => l.startsWith("profile chet: per-channel prompt ")), [
+    `profile chet: per-channel prompt "discord\\nstaged: /forged/bean.md" "1\\npairing: profile chet is Bean", ${Buffer.byteLength("marker-prompt-forge-4c1e")} bytes`,
+  ]);
+  assert.ok(!lines.some((l) => l.startsWith("staged: /forged") || l.startsWith("pairing: profile chet is Bean")), r.out);
+  assertNoText(s, r.all);
+});
+
 test("a read trace shows stage and the dry run open SOUL.md and config.yaml and never memory, sessions or secrets", (t) => {
   const s = scratch(t);
   for (const argv of [["--staging", s.staging], ["--dry-run"]]) {
@@ -288,23 +309,66 @@ test("install refuses an open or foreign staging file, a digest off the manifest
   assert.match(r.err, /the manifest doesn't list two personas/);
 });
 
-test("--dry-run writes nothing and prints only counts, two heading lines in a fixture counted", (t) => {
+test("--dry-run writes nothing and prints only counts and the fence fault, two heading lines in a fixture counted", (t) => {
   const s = scratch(t);
   personaTree(s, { chet: "You are Chet. marker-chet-dry-7d21\n# Voice\nShort.\r\n## Habits\nNot # a heading, nor #nospace.\n" });
   const before = readdirSync(s.root).sort();
   const r = run(["stage", "--hermes-root", s.hermes, ...PAIRS, "--dry-run"]);
-  assert.equal(r.status, 0, r.all);
+  assert.equal(r.status, 1, r.all);
   assert.deepEqual(readdirSync(s.root).sort(), before);
   assert.deepEqual(r.out.split("\n").filter(Boolean), [
     "files 2",
     'profile default: lines 3, lines starting "# " or "## " 0, lines naming a Hermes tool or channel 0',
     'profile chet: lines 5, lines starting "# " or "## " 2, lines naming a Hermes tool or channel 0',
+    'profile chet: persona holds </persona-file> or a line starting "# " or "## ", so C2-08\'s fence would refuse it',
   ]);
-  assert.equal(r.err, "");
+  assert.equal(r.err, "case2-persona-import: refused: C2-08's fence would refuse the persona of profile chet, so stage would write nothing\n");
   assertNoText(s, r.all);
   assert.ok(!r.all.includes("Voice") && !r.all.includes("Habits"));
 });
 
+// C2-20b N2 (webby-0073): a persona over the fence and under the byte cap used to pass the dry run
+// and then refuse at stage. The dry run names the fault and refuses too, and passes at the limit.
+test("--dry-run applies C2-08's fence: over it the fault prints, nothing is written and it exits 1", (t) => {
+  const atLimit = "é".repeat(PERSONA_CHARS);
+  const s = scratch(t);
+  personaTree(s, { bean: atLimit + "é" });
+  const before = readdirSync(s.root).sort();
+  const r = run(["stage", "--hermes-root", s.hermes, ...PAIRS, "--dry-run"]);
+  assert.equal(r.status, 1, r.all);
+  assert.deepEqual(readdirSync(s.root).sort(), before);
+  assert.deepEqual(r.out.split("\n").filter(Boolean), [
+    "files 2",
+    'profile default: lines 1, lines starting "# " or "## " 0, lines naming a Hermes tool or channel 0',
+    "profile default: persona is over 8000 characters, so C2-08's fence would refuse it",
+    'profile chet: lines 2, lines starting "# " or "## " 0, lines naming a Hermes tool or channel 0',
+  ]);
+  assert.match(r.err, /refused: C2-08's fence would refuse the persona of profile default, so stage would write nothing/);
+  assert.ok(!r.all.includes("éé"));
+  personaTree(s, { bean: atLimit });
+  const ok = run(["stage", "--hermes-root", s.hermes, ...PAIRS, "--dry-run"]);
+  assert.equal(ok.status, 0, ok.all);
+  assert.equal(ok.err, "");
+});
+
+// C2-20b N4 (webby-0073): a directory made between privateDir's lstat and its mkdir refuses with a
+// named reason, not a raw EEXIST. mkdir-race.mjs makes the directory just before the script's mkdir.
+test("a directory made between the check and mkdir refuses by name, at stage and at install", (t) => {
+  const s = scratch(t);
+  let r = stage(s, [], { race: s.staging });
+  assert.equal(r.status, 1, r.all);
+  assert.equal(r.err, `case2-persona-import: refused: ${s.staging} was made by something else between the check and mkdir\n`);
+  assert.deepEqual(readdirSync(s.staging), []);
+
+  const s2 = scratch(t);
+  assert.equal(stage(s2).status, 0);
+  const dir = join(s2.configDir, "personas", "case2");
+  r = install(s2, "case2", { race: dir });
+  assert.equal(r.status, 1, r.all);
+  assert.equal(r.err, `case2-persona-import: refused: ${dir} was made by something else between the check and mkdir\n`);
+  assert.deepEqual(readdirSync(dir), []);
+});
+
 test("--dry-run counts three lines naming a Hermes tool or channel and prints none of the words", (t) => {
   const s = scratch(t);
   personaTree(s, {

packages/cli/tests/fixtures/case2-persona-import/mkdir-race.mjs

@@ -0,0 +1,13 @@
+// mkdir race for case2-persona-import.test.mjs: loaded with --import, it makes the directory named by
+// $PERSONA_MKDIR_RACE just before the script's own mkdirSync of that path, as if another process won
+// the race after the script's lstat. The script's mkdir then meets a real EEXIST.
+import fs from "node:fs";
+import { syncBuiltinESMExports } from "node:module";
+
+const { mkdirSync } = fs;
+const target = process.env.PERSONA_MKDIR_RACE;
+fs.mkdirSync = function raced(p, ...rest) {
+  if (String(p) === target) mkdirSync(p, { mode: 0o700 });
+  return mkdirSync.call(this, p, ...rest);
+};
+syncBuiltinESMExports();

scripts/case2-persona-import.mjs

@@ -18,9 +18,11 @@
 // staged file against the manifest and C2-08's fence, and writes
 // <configDir>/personas/<business>/<instance>.md at 0600. It never overwrites.
 //
-// Output is names, byte counts and sha256, never persona or prompt text. --dry-run writes nothing
-// and prints counts only: files, lines, lines starting "# " or "## ", and lines naming a Hermes tool
-// or channel from HERMES_WORDS. A fence fault there is counted, not refused.
+// Output is names, byte counts and sha256, never persona or prompt text. A key from config.yaml
+// prints as a JSON string, so it can't start a line of its own. --dry-run writes nothing and prints
+// counts only: files, lines, lines starting "# " or "## ", and lines naming a Hermes tool or channel
+// from HERMES_WORDS. It applies C2-08's fence as stage does, prints each persona's fault by its
+// reason and then refuses. A SOUL.md over four times the fence in bytes refuses before any count.
 //
 // Exit codes: 0 ok, 1 refused, 2 usage.
 
@@ -184,7 +186,12 @@ function privateDir(path, { create }) {
   const s = lstatSync(path, { throwIfNoEntry: false });
   if (!s) {
     if (!create) throw new Refusal(`${path} doesn't exist`);
-    mkdirSync(path, { mode: 0o700 });
+    try {
+      mkdirSync(path, { mode: 0o700 });
+    } catch (e) {
+      if (e.code === "EEXIST") throw new Refusal(`${path} was made by something else between the check and mkdir`);
+      throw e;
+    }
     chmodSync(path, 0o700);
     return;
   }
@@ -221,10 +228,14 @@ function stage(o, say) {
 
   if (o.dryRun) {
     say(`files ${personas.length}`);
+    const faulted = [];
     for (const p of personas) {
       const c = counts(p.text);
       say(`profile ${p.profile}: lines ${c.lines}, lines starting "# " or "## " ${c.headings}, lines naming a Hermes tool or channel ${c.words}`);
+      const fault = personaTextFault(p.text);
+      if (fault) { say(`profile ${p.profile}: persona ${fault}, so C2-08's fence would refuse it`); faulted.push(p.profile); }
     }
+    if (faulted.length) throw new Refusal(`C2-08's fence would refuse the persona of profile ${faulted.join(" and ")}, so stage would write nothing`);
     return;
   }
 
@@ -234,7 +245,7 @@ function stage(o, say) {
   for (const p of pairs) {
     const prompts = config(p);
     if (!prompts.length) say(`profile ${p.profile}: no per-channel prompts`);
-    for (const c of prompts) say(`profile ${p.profile}: per-channel prompt ${c.where} ${c.channel}, ${c.bytes === null ? "not text" : `${c.bytes} bytes`}`);
+    for (const c of prompts) say(`profile ${p.profile}: per-channel prompt ${JSON.stringify(c.where)} ${JSON.stringify(c.channel)}, ${c.bytes === null ? "not text" : `${c.bytes} bytes`}`);
   }
   for (const p of personas) {
     const fault = personaTextFault(p.text);