From 865386d2eb226c6a976a4ceb4500fbf62f86622c Mon Sep 17 00:00:00 2001 From: NeilJo-GY <43027886+NeilJo-GY@users.noreply.github.com> Date: Sun, 19 Jul 2026 15:27:58 +0800 Subject: [PATCH 1/2] fix(feedback): prefer explicit env over implicit ~/.openclaw Align refine.js and state-sync.template.js so OPENCLAW_HOME / OPENPERSONA_HOME win before default-directory discovery, and lock the precedence with unit + generated-script regression tests. Co-authored-by: Cursor --- layers/body/SIGNAL-PROTOCOL.md | 27 +++++++++---- lib/lifecycle/refine.js | 20 +++++++--- templates/body/state-sync.template.js | 22 ++++++++--- tests/generator-state-sync.test.js | 36 +++++++++++++++++ tests/refine.test.js | 56 +++++++++++++++++++++++---- 5 files changed, 136 insertions(+), 25 deletions(-) diff --git a/layers/body/SIGNAL-PROTOCOL.md b/layers/body/SIGNAL-PROTOCOL.md index 494bf8d..4c3e6ce 100644 --- a/layers/body/SIGNAL-PROTOCOL.md +++ b/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. + ``` / ├── 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'); diff --git a/lib/lifecycle/refine.js b/lib/lifecycle/refine.js index 5b9b952..4e99041 100644 --- a/lib/lifecycle/refine.js +++ b/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, diff --git a/templates/body/state-sync.template.js b/templates/body/state-sync.template.js index 9ee0f2c..91848d4 100644 --- a/templates/body/state-sync.template.js +++ b/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'); diff --git a/tests/generator-state-sync.test.js b/tests/generator-state-sync.test.js index fcabc1c..cdac685 100644 --- a/tests/generator-state-sync.test.js +++ b/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); }); diff --git a/tests/refine.test.js b/tests/refine.test.js index 052b9b8..3b19cb2 100644 --- a/tests/refine.test.js +++ b/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')); }); }); From f28e63179f614003cdee707878b79f94595f26bc Mon Sep 17 00:00:00 2001 From: NeilJo-GY <43027886+NeilJo-GY@users.noreply.github.com> Date: Sun, 19 Jul 2026 15:54:44 +0800 Subject: [PATCH 2/2] fix(ci): retry IPC deserialize flakes up to 3 times The skill-v0.21.1-fixes suite still flakes under node --test IPC on GitHub runners; one retry was not enough. Also disable matrix fail-fast so one Node version does not cancel the others. Co-authored-by: Cursor --- .github/workflows/ci.yml | 1 + scripts/run-tests.js | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 553fcbb..fec2252 100644 --- a/.github/workflows/ci.yml +++ b/.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"] diff --git a/scripts/run-tests.js b/scripts/run-tests.js index ee76cfc..50f5cf3 100644 --- a/scripts/run-tests.js +++ b/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);