Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -713,3 +713,53 @@ Cancel → approved → `openclaw config get gateway.auth.mode` returned **token

That is the whole chain — detect, propose, review, approve, apply, verify — fixing a real problem
on a real OpenClaw install through the shipped interface.


---

# Fifth pass — more repairs, and honest finding titles

## Repair catalog: 1 → 5

Three more config repairs, all built on `openclaw config set` with the read-back as verification,
all sharing one `configToggleRepair` contract so the guard rails cannot drift apart:

| Repair | Key | Verified on a real install |
|---|---|---|
| `auto-update-enabled-warning` | `update.auto.enabled` → false | applied, finding gone |
| `gateway-loopback-no-auth` | `gateway.auth.mode` → token | applied, finding gone |
| `no-hybrid-search` | `agents.defaults.memorySearch.query.hybrid.enabled` → true | applied, `current:"true"`, finding gone |
| `no-memory-flush` | `agents.defaults.compaction.memoryFlush.enabled` → true | applied, `current:"true"`, finding gone |

Four of the five now work without a service manager, so a containerised OpenClaw is no longer a
host where ClawFix can only look and shrug. `gateway-not-running` still needs systemd or launchd —
that is OpenClaw's restart path, not something ClawFix can route around.

Each toggle refuses an unreadable key (`config_state_unknown`) rather than treating "cannot read"
as "false", and rollback restores the previous value.

## Port conflict: deliberately still not repaired

The obvious next repair is the port conflict — ClawFix already knows the squatting PID. It was not
added, because every version of it ends in killing a process ClawFix does not own. A repair that
terminates an unrelated service is not a guarded repair, whatever the preview says. If this is
wanted later, the safe shape is: refuse unless the listener is itself a stale OpenClaw process,
and even then prefer OpenClaw's own `--force` restart over a kill.

## Two findings were shouting machine text at the user

`[critical] timeout` came from `nativeStatus.gateway.error` being used directly as an issue title.
A probe that gave up was presented as a critical OpenClaw fault named "timeout". The same pattern
turned a validator message into the title `JSON5 parse failed: SyntaxError: JSON5: invalid
character 'i' at 1:8`.

Both now carry an actionable headline with the upstream text as detail:

```
[high] OpenClaw config schema validation failed
detail: JSON5 parse failed: SyntaxError: JSON5: invalid character 'i' at 1:8
```

Fixing the title broke the doctor-findings dedup, which had been matching on the raw message — so
the same problem briefly appeared twice, once well-titled and once raw. Caught on the real
machine, not in tests: the dedup now compares the detail as well as the headline.
23 changes: 20 additions & 3 deletions cli/core/diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,14 @@ export function deriveIssues(collected) {
}

if (nativeConfig.available && nativeConfig.valid === false) {
// The headline stays a sentence the operator can act on; the validator's own text is
// detail, not the title. Surfacing it raw produced findings titled
// "JSON5 parse failed: SyntaxError: JSON5: invalid character 'i' at 1:8".
const configDetail = nativeConfig.errors[0]?.message || '';
issues.push({
severity: 'high',
text: nativeConfig.errors[0]?.message || 'OpenClaw config schema validation failed',
text: 'OpenClaw config schema validation failed',
description: configDetail || 'OpenClaw reported the config as invalid without detail.',
source: 'openclaw-config',
nativeCheckId: 'config/schema-invalid',
path: nativeConfig.errors[0]?.path || null,
Expand All @@ -385,9 +390,16 @@ export function deriveIssues(collected) {
&& nativeStatus.gateway.reachable === false
&& !issues.some((issue) => /gateway.*not running|gateway.*unreachable/i.test(issue.text))
) {
// Same rule as the config case above. `gateway.error` is a machine string — when the probe
// timed out it was literally "timeout", which reached the user as a critical finding titled
// "timeout" and read like an OpenClaw fault rather than a probe that gave up.
const probeDetail = String(nativeStatus.gateway.error || '').trim();
issues.push({
severity: 'critical',
text: nativeStatus.gateway.error || 'OpenClaw gateway is unreachable',
text: 'OpenClaw gateway is unreachable',
description: probeDetail
? `Gateway probe failed: ${probeDetail}`
: 'The gateway did not answer a status probe.',
source: 'openclaw-status',
nativeCheckId: 'status/gateway-unreachable',
});
Expand Down Expand Up @@ -423,9 +435,14 @@ export function deriveIssues(collected) {
}

for (const finding of nativeDoctor.findings) {
// Match on the detail too, not just the headline. Findings that carry an upstream message as
// their detail (config/schema-invalid holds the validator's text) would otherwise be
// reported twice — once with an actionable title, once as the raw parser output.
const message = String(finding.message || '').toLowerCase();
const duplicate = issues.some((issue) => (
issue.nativeCheckId === finding.checkId
|| issue.text.toLowerCase() === finding.message.toLowerCase()
|| String(issue.text || '').toLowerCase() === message
|| String(issue.description || '').toLowerCase() === message
));
if (duplicate) continue;
issues.push({
Expand Down
141 changes: 94 additions & 47 deletions cli/core/repair-catalog.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,60 +108,105 @@ function configFlag(value) {
return null;
}

const autoUpdateEnabled = Object.freeze({
/**
* A repair that flips one boolean OpenClaw config key to `target`.
*
* Every step goes through the OpenClaw CLI as argv — read the current value, set it, read it
* back. Verification is the read-back, so a `config set` that reported success but changed
* nothing still fails. A key OpenClaw cannot report is refused rather than assumed: unreadable
* is not the same as false.
*/
function configToggleRepair({ id, key, target, title, description, blockedReason, risk = 'low' }) {
const targetText = String(target);
const previousText = String(!target);

return Object.freeze({
id,
key,
title,
description,
risk,

async preflight(ctx) {
const current = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 });
const flag = configFlag(current);
if (flag === null) {
return Object.freeze({ ok: false, reason: 'config_state_unknown', evidence: { key, current } });
}
if (flag === target) {
return Object.freeze({ ok: false, reason: blockedReason, evidence: { key, current } });
}
return Object.freeze({ ok: true, evidence: { key, current } });
},

async preview() {
return Object.freeze({
steps: Object.freeze([
`Read ${key} through the OpenClaw CLI (argv, no shell).`,
`Set ${key} to ${targetText}.`,
'Read it back to confirm the change actually landed.',
]),
summary: `openclaw config set ${key} ${targetText}`,
});
},

async apply(ctx) {
const result = await ctx.openclaw.configSet(key, targetText, { timeoutMs: 30_000 });
return Object.freeze({
status: result.status,
timedOut: result.timedOut,
errorSummary: result.errorSummary,
});
},

async verify(ctx) {
const current = await ctx.openclaw.configGet(key, { timeoutMs: 10_000 });
return Object.freeze({ ok: configFlag(current) === target, evidence: { key, current } });
},

async rollback(ctx) {
const result = await ctx.openclaw.configSet(key, previousText, { timeoutMs: 30_000 });
return Object.freeze({
rolledBack: result.status === 0,
note: result.status === 0
? `Restored ${key} to ${previousText}.`
: `Could not restore ${key}; check \`openclaw config get ${key}\`.`,
});
},
});
}

const autoUpdateEnabled = configToggleRepair({
id: 'auto-update-enabled-warning',
key: 'update.auto.enabled',
target: false,
title: 'Disable OpenClaw auto-update',
description:
'Auto-update restarts the gateway on its own schedule, which is the documented cause of '
+ 'restart loops. This turns it off; updates then happen when you run them.',
risk: 'low',

async preflight(ctx) {
const current = await ctx.openclaw.configGet('update.auto.enabled', { timeoutMs: 10_000 });
const enabled = configFlag(current);
if (enabled === null) {
return Object.freeze({ ok: false, reason: 'auto_update_state_unknown', evidence: { current } });
}
if (enabled === false) {
return Object.freeze({ ok: false, reason: 'auto_update_already_disabled', evidence: { current } });
}
return Object.freeze({ ok: true, evidence: { current } });
},

async preview() {
return Object.freeze({
steps: Object.freeze([
'Read update.auto.enabled through the OpenClaw CLI (argv, no shell).',
'Set update.auto.enabled to false.',
'Read it back to confirm it is off.',
]),
summary: 'openclaw config set update.auto.enabled false',
});
},

async apply(ctx) {
const result = await ctx.openclaw.configSet('update.auto.enabled', 'false', { timeoutMs: 30_000 });
return Object.freeze({
status: result.status,
timedOut: result.timedOut,
errorSummary: result.errorSummary,
});
},
blockedReason: 'auto_update_already_disabled',
});

async verify(ctx) {
const current = await ctx.openclaw.configGet('update.auto.enabled', { timeoutMs: 10_000 });
return Object.freeze({ ok: configFlag(current) === false, evidence: { current } });
},
const hybridSearchDisabled = configToggleRepair({
id: 'no-hybrid-search',
key: 'agents.defaults.memorySearch.query.hybrid.enabled',
target: true,
title: 'Enable hybrid memory search',
description:
'Hybrid search combines keyword and semantic matching when the agent searches memory. '
+ 'OpenClaw recommends it; without it recall is keyword-only.',
blockedReason: 'hybrid_search_already_enabled',
});

async rollback(ctx) {
const result = await ctx.openclaw.configSet('update.auto.enabled', 'true', { timeoutMs: 30_000 });
return Object.freeze({
rolledBack: result.status === 0,
note: result.status === 0
? 'Restored update.auto.enabled to true.'
: 'Could not restore update.auto.enabled; check `openclaw config get update.auto.enabled`.',
});
},
const memoryFlushDisabled = configToggleRepair({
id: 'no-memory-flush',
key: 'agents.defaults.compaction.memoryFlush.enabled',
target: true,
title: 'Enable memory flush on compaction',
description:
'Without a memory flush, anything the agent has not written down is lost when the context '
+ 'is compacted. This makes compaction persist memory first.',
blockedReason: 'memory_flush_already_enabled',
});

const gatewayLoopbackNoAuth = Object.freeze({
Expand Down Expand Up @@ -234,4 +279,6 @@ export const repairCatalog = Object.freeze({
'gateway-not-running': gatewayNotRunning,
'auto-update-enabled-warning': autoUpdateEnabled,
'gateway-loopback-no-auth': gatewayLoopbackNoAuth,
'no-hybrid-search': hybridSearchDisabled,
'no-memory-flush': memoryFlushDisabled,
});
63 changes: 62 additions & 1 deletion test/diagnostic-core.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,9 @@ test('deriveIssues golden: comprehensive representative facts produce the exact
},
{
severity: 'high',
text: 'Gateway port must be an integer',
// The validator's own message is detail; the headline stays actionable.
text: 'OpenClaw config schema validation failed',
description: 'Gateway port must be an integer',
source: 'openclaw-config',
nativeCheckId: 'config/schema-invalid',
path: 'gateway.port',
Expand Down Expand Up @@ -2718,3 +2720,62 @@ test('non-Error external abort reasons use DiagnosticsAbortError', async () => {
);
assert.equal(fake.clearCalls.length, 1);
});

// ============================================================
// Raw upstream error strings must not become finding titles
// ============================================================

test('a gateway probe timeout is reported as unreachable, not as a finding titled "timeout"', () => {
// A competing port owner suppresses the generic not-running issue, so the unreachable
// branch is the one under test here.
const issues = deriveIssues(fakeCollected({
gateway: { gatewayStatus: 'not running', gatewayPort: 4321, gatewayPid: '' },
portsGateway: {
valid: true, available: true, listening: true, process: 'some-other-process', pid: 777, collector: 'lsof',
},
nativeStatus: { available: true, gateway: { reachable: false, error: 'timeout' } },
}));

const unreachable = issues.find(issue => issue.nativeCheckId === 'status/gateway-unreachable');
assert.ok(unreachable, 'expected an unreachable finding');
assert.equal(unreachable.text, 'OpenClaw gateway is unreachable');
assert.match(unreachable.description, /Gateway probe failed: timeout/);
// The bare machine string must never be the headline.
assert.equal(issues.some(issue => issue.text === 'timeout'), false);
});

test('an invalid config keeps the parser message as detail, not as the title', () => {
const issues = deriveIssues(fakeCollected({
nativeConfig: {
available: true,
valid: false,
errors: [{ message: "JSON5 parse failed: SyntaxError: JSON5: invalid character 'i' at 1:8", path: 'gateway' }],
},
}));

const invalid = issues.find(issue => issue.nativeCheckId === 'config/schema-invalid');
assert.ok(invalid, 'expected a config finding');
assert.equal(invalid.text, 'OpenClaw config schema validation failed');
assert.match(invalid.description, /JSON5 parse failed/);
assert.equal(issues.some(issue => /^JSON5 parse failed/.test(issue.text)), false);
});

test('a doctor finding that repeats an already-reported detail is not listed twice', () => {
const parserMessage = "JSON5 parse failed: SyntaxError: JSON5: invalid character 'i' at 1:8";
const issues = deriveIssues(fakeCollected({
nativeConfig: {
available: true,
valid: false,
errors: [{ message: parserMessage, path: 'gateway' }],
},
nativeDoctor: {
available: true,
findings: [{ checkId: 'core/doctor/config', severity: 'error', message: parserMessage, path: 'gateway' }],
},
}));

const mentions = issues.filter(issue =>
issue.text === parserMessage || issue.description === parserMessage);
assert.equal(mentions.length, 1, 'the parser message must appear once');
assert.equal(mentions[0].text, 'OpenClaw config schema validation failed');
});
Loading