fix(feedback): prefer explicit env over implicit ~/.openclaw

.github/workflows/ci.yml

@@ -11,6 +11,7 @@ jobs:
     name: Tests (Node ${{ matrix.node }})
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         node: ["18", "20", "22"]
 

layers/body/SIGNAL-PROTOCOL.md

@@ -46,10 +46,13 @@ Both files live in a **feedback directory** resolved from the host's home locati
 The persona's `state-sync.js` resolves the path in this order:
 
 1. `$OPENCLAW_HOME/feedback/` — if env var `OPENCLAW_HOME` is set
-2. `~/.openclaw/feedback/` — if that directory already exists (standard OpenClaw layout)
-3. `$OPENPERSONA_HOME/feedback/` — explicit override for non-OpenClaw runners
+2. `$OPENPERSONA_HOME/feedback/` — if env var `OPENPERSONA_HOME` is set (explicit non-OpenClaw / test override)
+3. `~/.openclaw/feedback/` — if that directory already exists (standard OpenClaw layout)
 4. `~/.openpersona/feedback/` — universal fallback
 
+Explicit environment variables always win over implicit directory discovery. In particular,
+setting `OPENPERSONA_HOME` must not be shadowed by a pre-existing `~/.openclaw` on the machine.
+
 ```
 <feedback-dir>/
 ├── signals.json          ← persona writes here (array, capped at 200 entries)
@@ -396,11 +399,21 @@ const path = require('path');
 const os = require('os');
 
 // Resolve feedback dir — mirror the same logic as state-sync.js
-const OPENCLAW_DIR = process.env.OPENCLAW_HOME || path.join(os.homedir(), '.openclaw');
-const FALLBACK_DIR = process.env.OPENPERSONA_HOME || path.join(os.homedir(), '.openpersona');
-const FEEDBACK_DIR = (process.env.OPENCLAW_HOME || fs.existsSync(OPENCLAW_DIR))
-  ? path.join(OPENCLAW_DIR, 'feedback')
-  : path.join(FALLBACK_DIR, 'feedback');
+// Explicit env wins: OPENCLAW_HOME → OPENPERSONA_HOME → ~/.openclaw → ~/.openpersona
+function resolveFeedbackDir() {
+  if (process.env.OPENCLAW_HOME) {
+    return path.join(process.env.OPENCLAW_HOME, 'feedback');
+  }
+  if (process.env.OPENPERSONA_HOME) {
+    return path.join(process.env.OPENPERSONA_HOME, 'feedback');
+  }
+  const clawHome = path.join(os.homedir(), '.openclaw');
+  const opHome   = path.join(os.homedir(), '.openpersona');
+  return fs.existsSync(clawHome)
+    ? path.join(clawHome, 'feedback')
+    : path.join(opHome, 'feedback');
+}
+const FEEDBACK_DIR = resolveFeedbackDir();
 
 const SIGNALS_PATH = path.join(FEEDBACK_DIR, 'signals.json');
 const RESPONSES_PATH = path.join(FEEDBACK_DIR, 'signal-responses.json');

lib/lifecycle/refine.js

@@ -157,13 +157,22 @@ function applySkillGates(personaDir, persona, newEvents) {
 }
 
 // ── Feedback directory resolution (mirrors state-sync.js logic) ─────────────
+// Explicit env wins over implicit default-directory discovery:
+//   OPENCLAW_HOME → OPENPERSONA_HOME → ~/.openclaw (if exists) → ~/.openpersona
+// Injected deps keep unit tests hermetic (real os.homedir / existsSync in production).
 
-function resolveFeedbackDir() {
-  const clawHome = process.env.OPENCLAW_HOME || path.join(os.homedir(), '.openclaw');
-  const opHome   = process.env.OPENPERSONA_HOME || path.join(os.homedir(), '.openpersona');
-  return (process.env.OPENCLAW_HOME || fs.existsSync(clawHome))
+function resolveFeedbackDir(env = process.env, homedir = os.homedir(), existsSync = fs.existsSync) {
+  if (env.OPENCLAW_HOME) {
+    return path.join(env.OPENCLAW_HOME, 'feedback');
+  }
+  if (env.OPENPERSONA_HOME) {
+    return path.join(env.OPENPERSONA_HOME, 'feedback');
+  }
+  const clawHome = path.join(homedir, '.openclaw');
+  const opHome   = path.join(homedir, '.openpersona');
+  return existsSync(clawHome)
     ? path.join(clawHome, 'feedback')
-    : path.join(opHome,   'feedback');
+    : path.join(opHome, 'feedback');
 }
 
 // ── Pack regeneration (Social + SKILL.md sync) ──────────────────────────────
@@ -522,6 +531,7 @@ module.exports = {
   refine,
   emitRefinement,
   applyRefinement,
+  resolveFeedbackDir,
   scanConstitutionKeywords,
   loadMeta,
   writeMeta,

scripts/run-tests.js

@@ -41,6 +41,7 @@ if (files.length === 0) {
 }
 
 const DESERIALIZE_RE = /Unable to deserialize cloned data/;
+const MAX_DESERIALIZE_ATTEMPTS = 3;
 
 function runTestFile(file) {
   return spawnSync(
@@ -50,12 +51,23 @@ function runTestFile(file) {
   );
 }
 
+function isDeserializeFlake(result) {
+  const combined = `${result.stdout || ''}${result.stderr || ''}`;
+  return (result.status ?? 1) !== 0 && DESERIALIZE_RE.test(combined);
+}
+
 for (const file of files) {
   const rel = path.relative(root, file);
   let result = runTestFile(file);
-  const combined = `${result.stdout || ''}${result.stderr || ''}`;
-  if ((result.status ?? 1) !== 0 && DESERIALIZE_RE.test(combined)) {
-    process.stderr.write(`\n[retry] IPC deserialize flake in ${rel}, re-running once…\n`);
+  let attempt = 1;
+  while (isDeserializeFlake(result) && attempt < MAX_DESERIALIZE_ATTEMPTS) {
+    attempt += 1;
+    process.stderr.write(
+      `\n[retry] IPC deserialize flake in ${rel}, re-running (${attempt}/${MAX_DESERIALIZE_ATTEMPTS})…\n`
+    );
+    // Brief pause — back-to-back worker respawns are more likely to hit the same race.
+    const pauseUntil = Date.now() + 200;
+    while (Date.now() < pauseUntil) { /* spin */ }
     result = runTestFile(file);
   }
   if (result.stdout) process.stdout.write(result.stdout);

templates/body/state-sync.template.js

@@ -16,12 +16,22 @@ const os = require('os');
 
 const PERSONA_DIR = path.resolve(__dirname, '..');
 const STATE_PATH = path.join(PERSONA_DIR, 'state.json');
-// Signals: use OPENCLAW_HOME if explicitly set or ~/.openclaw exists; else fall back to ~/.openpersona
-const OPENCLAW_DIR = process.env.OPENCLAW_HOME || path.join(os.homedir(), '.openclaw');
-const PERSONA_DIR_BASE = process.env.OPENPERSONA_HOME || path.join(os.homedir(), '.openpersona');
-const FEEDBACK_DIR = (process.env.OPENCLAW_HOME || fs.existsSync(OPENCLAW_DIR))
-  ? path.join(OPENCLAW_DIR, 'feedback')
-  : path.join(PERSONA_DIR_BASE, 'feedback');
+// Signals: explicit env wins over implicit default-directory discovery
+//   OPENCLAW_HOME → OPENPERSONA_HOME → ~/.openclaw (if exists) → ~/.openpersona
+function resolveFeedbackDir() {
+  if (process.env.OPENCLAW_HOME) {
+    return path.join(process.env.OPENCLAW_HOME, 'feedback');
+  }
+  if (process.env.OPENPERSONA_HOME) {
+    return path.join(process.env.OPENPERSONA_HOME, 'feedback');
+  }
+  const clawHome = path.join(os.homedir(), '.openclaw');
+  const opHome   = path.join(os.homedir(), '.openpersona');
+  return fs.existsSync(clawHome)
+    ? path.join(clawHome, 'feedback')
+    : path.join(opHome, 'feedback');
+}
+const FEEDBACK_DIR = resolveFeedbackDir();
 const SIGNALS_PATH = path.join(FEEDBACK_DIR, 'signals.json');
 const SIGNAL_RESPONSES_PATH = path.join(FEEDBACK_DIR, 'signal-responses.json');
 

tests/generator-state-sync.test.js

@@ -33,6 +33,42 @@ describe('state-sync script generation', () => {
     assert.ok(content.includes('writeState'), 'script must contain writeState function');
     assert.ok(content.includes('emitSignal'), 'script must contain emitSignal function');
     assert.ok(content.includes('capability_gap'), 'script must list valid signal types');
+    assert.ok(
+      content.includes('OPENPERSONA_HOME') && content.includes('resolveFeedbackDir'),
+      'script must resolve feedback dir with explicit OPENPERSONA_HOME precedence'
+    );
+
+    await fs.remove(TMP_SS);
+  });
+
+  it('state-sync.js prefers OPENPERSONA_HOME over implicit ~/.openclaw', async () => {
+    const persona = {
+      personaName: 'FeedbackHomeTest',
+      slug: 'feedback-home-test',
+      bio: 'feedback home precedence tester',
+      personality: 'precise',
+      speakingStyle: 'Direct',
+    };
+    await fs.ensureDir(TMP_SS);
+    const { skillDir } = await generate(persona, TMP_SS);
+
+    const { execSync } = require('child_process');
+    const syncScript = path.join(skillDir, 'scripts', 'state-sync.js');
+    const opHome = path.join(TMP_SS, 'op-home-explicit');
+    const env = { ...process.env, OPENPERSONA_HOME: opHome };
+    delete env.OPENCLAW_HOME;
+
+    execSync(`node "${syncScript}" signal capability_gap '{"need":"test"}'`, {
+      encoding: 'utf-8',
+      cwd: skillDir,
+      env,
+    });
+
+    const signalsPath = path.join(opHome, 'feedback', 'signals.json');
+    assert.ok(
+      fs.existsSync(signalsPath),
+      'signals.json must land under OPENPERSONA_HOME even when ~/.openclaw exists on the machine'
+    );
 
     await fs.remove(TMP_SS);
   });

tests/refine.test.js

@@ -29,6 +29,7 @@ const {
   refine,
   emitRefinement,
   applyRefinement,
+  resolveFeedbackDir,
 } = require('../lib/lifecycle/refine');
 const { forkPersona } = require('../lib/lifecycle/forker');
 const { generate }    = require('../lib/generator');
@@ -268,7 +269,7 @@ describe('applyRefinement — compliance gate', () => {
     fs.writeFileSync(path.join(skillDir, 'state.json'), JSON.stringify({ eventLog: [] }));
 
     // Write signal-responses.json with a violating behavior guide into the feedback dir
-    // resolveFeedbackDir() checks OPENCLAW_HOME → falls back to OPENPERSONA_HOME
+    // resolveFeedbackDir() prefers explicit OPENPERSONA_HOME when OPENCLAW_HOME is unset
     const feedbackHome = path.join(TMP, 'compliance-home');
     const feedbackDir  = path.join(feedbackHome, 'feedback');
     fs.ensureDirSync(feedbackDir);
@@ -287,13 +288,54 @@ describe('applyRefinement — compliance gate', () => {
     process.env.OPENPERSONA_HOME = feedbackHome;
     delete process.env.OPENCLAW_HOME;
 
-    // Call applyRefinement directly with personaDir — no resolvePersonaDir involved
-    const result = await applyRefinement(skillDir, persona);
-    assert.strictEqual(result.applied, false);
-    assert.ok(Array.isArray(result.violations) && result.violations.length > 0);
+    try {
+      // Call applyRefinement directly with personaDir — no resolvePersonaDir involved
+      const result = await applyRefinement(skillDir, persona);
+      assert.strictEqual(result.applied, false);
+      assert.ok(Array.isArray(result.violations) && result.violations.length > 0);
+    } finally {
+      if (origHome === undefined) delete process.env.OPENPERSONA_HOME;
+      else process.env.OPENPERSONA_HOME = origHome;
+      if (origClaw === undefined) delete process.env.OPENCLAW_HOME;
+      else process.env.OPENCLAW_HOME = origClaw;
+    }
+  });
+});
+
+// ── resolveFeedbackDir precedence ────────────────────────────────────────────
+
+describe('resolveFeedbackDir — env precedence', () => {
+  const fakeHome = path.join(TMP, 'feedback-homedir');
+  const clawDefault = path.join(fakeHome, '.openclaw');
+  const opDefault   = path.join(fakeHome, '.openpersona');
+
+  it('prefers explicit OPENCLAW_HOME over everything', () => {
+    const dir = resolveFeedbackDir(
+      { OPENCLAW_HOME: '/explicit/claw', OPENPERSONA_HOME: '/explicit/op' },
+      fakeHome,
+      (p) => p === clawDefault
+    );
+    assert.strictEqual(dir, path.join('/explicit/claw', 'feedback'));
+  });
+
+  it('prefers explicit OPENPERSONA_HOME over implicit ~/.openclaw', () => {
+    // Regression: machines with ~/.openclaw must not shadow OPENPERSONA_HOME
+    const dir = resolveFeedbackDir(
+      { OPENPERSONA_HOME: '/explicit/op' },
+      fakeHome,
+      (p) => p === clawDefault
+    );
+    assert.strictEqual(dir, path.join('/explicit/op', 'feedback'));
+  });
+
+  it('falls back to ~/.openclaw when it exists and no env is set', () => {
+    const dir = resolveFeedbackDir({}, fakeHome, (p) => p === clawDefault);
+    assert.strictEqual(dir, path.join(clawDefault, 'feedback'));
+  });
 
-    process.env.OPENPERSONA_HOME = origHome;
-    if (origClaw !== undefined) process.env.OPENCLAW_HOME = origClaw;
+  it('falls back to ~/.openpersona when no env and ~/.openclaw is absent', () => {
+    const dir = resolveFeedbackDir({}, fakeHome, () => false);
+    assert.strictEqual(dir, path.join(opDefault, 'feedback'));
   });
 });