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
57 changes: 57 additions & 0 deletions REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -656,3 +656,60 @@ invest next — port-conflict and config-invalid are both detected precisely eno
- Raw parser output is used as a user-facing title: `JSON5 parse failed: SyntaxError: JSON5:
invalid character 'i' at 1:8`. Accurate, but it is a stack-trace fragment where a sentence
belongs.


---

# Fourth pass — the repair catalog, and driving the real UI

## Two repairs added: ClawFix now fixes what it finds

The catalog held one repair that needed a service manager. Two more were added, both using
OpenClaw's own supported commands and both verified against a real OpenClaw 2026.6.11 install:

| Repair | Does | Verified by | Risk |
|---|---|---|---|
| `auto-update-enabled-warning` | `openclaw config set update.auto.enabled false` | reads the key back through `config get` | low |
| `gateway-loopback-no-auth` | sets `gateway.auth.mode` to `token`, then `doctor --fix --generate-gateway-token` | reads the mode back; never reads the token itself | medium |

Repairable findings went from 1 to 3. On a real install, broken deliberately:

```
auto-update: before "true" → applied, verify {"ok":true,"current":"false"} → finding gone
gateway auth: before "none" → applied, verify {"ok":true,"mode":"token"} → finding gone
```

`gateway-loopback-no-auth` is deliberately medium risk, not low: existing clients stop working
until they carry the new token, and the preview says so.

Verification reads one config key through OpenClaw rather than parsing `openclaw.json`, so repair
evidence stays narrow — and the auth repair never pulls the token into a repair record.

## Driving the real UI found two defects

The compiled musl binary was driven through a PTY with a terminal emulator attached, on a real
broken install. Both of these were invisible to unit tests and frame captures.

**The sidebar's numbers did not match the numbers `fix <#>` accepts.** The sidebar sorted findings
by severity and renumbered its own view 1–4, while `fix <#>` and `explain <#>` index the unsorted
findings list. Reading *"3. Auto-update enabled"* and typing `fix 3` reached an advisory finding
and answered "This finding has no reviewed automatic repair." The sidebar now carries each
finding's real position — criticals still lead, but the numbers are the ones the commands take.

**The status line printed the revision twice**, which pushed the finding count off the end:

```
before: 🦞 ClawFix v0.11.2 · revision f3e9479b-… · Revision f3e9479b-…
after: 🦞 ClawFix v0.11.2 · Revision f3e9479b-b52c-456e-8d0f-f5f25ba3b2e4 · 6 findings · AI consent required
```

The bridge's status already begins with the revision; the extra prefix was redundant.

## End to end through the UI, on a real machine

Sidebar listed *"4. Gateway auth missing on loopback"* → typed `fix 4` → approval dialog showed
`Risk: medium · gateway-loopback-no-auth` with the composer locked and focus defaulting to
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.
20 changes: 20 additions & 0 deletions cli/adapters/openclaw.js
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,26 @@ export function createOpenClawAdapter({
npmVersion(options = {}) {
return successfulText('npm', ['--version'], options);
},

/**
* Read one config key through OpenClaw itself.
*
* Repairs verify against this rather than parsing openclaw.json: it is the value OpenClaw
* resolves, and it keeps repair evidence to a single key instead of a whole config blob.
* Returns '' when the key is unset or the call fails — callers must not read that as false.
*/
async configGet(key, options = {}) {
if (typeof key !== 'string' || !/^[A-Za-z0-9_.-]{1,128}$/.test(key)) return '';
return processText(await invoke(['config', 'get', key], options));
},

/** Set one config key. Values are passed as literal argv, never through a shell. */
async configSet(key, value, options = {}) {
if (typeof key !== 'string' || !/^[A-Za-z0-9_.-]{1,128}$/.test(key)) {
return Object.freeze({ status: 1, errorSummary: 'invalid config key' });
}
return invoke(['config', 'set', key, String(value)], options);
},
/**
* PIDs that plausibly belong to a running gateway *server*.
*
Expand Down
1 change: 1 addition & 0 deletions cli/core/findings.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const LOCAL_KNOWN_ISSUE_ID_REPAIR_MAP = new Map([]);
// Explicit native checkId -> repairId map (exact checkId equality only).
const NATIVE_CHECK_ID_REPAIR_MAP = new Map([
['runtime/gateway-port-conflict', 'port-conflict'],
['gateway.loopback_no_auth', 'gateway-loopback-no-auth'],
]);

function slug(value) {
Expand Down
132 changes: 132 additions & 0 deletions cli/core/repair-catalog.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,138 @@ const gatewayNotRunning = Object.freeze({
},
});

/** Normalized truthiness of an `openclaw config get` result. '' means unset or unreadable. */
function configFlag(value) {
const text = String(value ?? '').trim().toLowerCase();
if (text === 'true') return true;
if (text === 'false') return false;
return null;
}

const autoUpdateEnabled = Object.freeze({
id: 'auto-update-enabled-warning',
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,
});
},

async verify(ctx) {
const current = await ctx.openclaw.configGet('update.auto.enabled', { timeoutMs: 10_000 });
return Object.freeze({ ok: configFlag(current) === false, evidence: { current } });
},

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 gatewayLoopbackNoAuth = Object.freeze({
id: 'gateway-loopback-no-auth',
title: 'Require a token on the gateway',
description:
'The gateway accepts unauthenticated connections. This switches auth to token mode and has '
+ 'OpenClaw generate one. Clients will need that token to connect afterwards.',
// Medium, not low: existing clients stop working until they carry the new token.
risk: 'medium',

async preflight(ctx) {
const mode = await ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 });
const current = String(mode || '').trim().toLowerCase();
if (current === 'token' || current === 'password' || current === 'trusted-proxy') {
return Object.freeze({ ok: false, reason: 'gateway_auth_already_enabled', evidence: { mode: current } });
}
return Object.freeze({ ok: true, evidence: { mode: current || '(unset)' } });
},

async preview() {
return Object.freeze({
steps: Object.freeze([
'Set gateway.auth.mode to token through the OpenClaw CLI (argv, no shell).',
'Run `openclaw doctor --fix --generate-gateway-token` so OpenClaw generates the token.',
'Read gateway.auth.mode back to confirm token auth is active.',
'Restart the gateway yourself for it to take effect; existing clients need the new token.',
]),
summary: 'openclaw config set gateway.auth.mode token + doctor --generate-gateway-token',
});
},

async apply(ctx) {
const set = await ctx.openclaw.configSet('gateway.auth.mode', 'token', { timeoutMs: 30_000 });
if (set.status !== 0) {
return Object.freeze({ status: set.status, stage: 'set-mode', errorSummary: set.errorSummary });
}
const generated = await ctx.openclaw.invoke(
['doctor', '--fix', '--generate-gateway-token'],
{ timeoutMs: 120_000 },
);
return Object.freeze({
status: generated.status,
stage: 'generate-token',
timedOut: generated.timedOut,
errorSummary: generated.errorSummary,
});
},

async verify(ctx) {
// Evidence is the mode only — never read the token itself into a repair record.
const mode = String(await ctx.openclaw.configGet('gateway.auth.mode', { timeoutMs: 10_000 })).trim();
return Object.freeze({ ok: mode.toLowerCase() === 'token', evidence: { mode } });
},

async rollback(ctx, { applyResult } = {}) {
if (applyResult?.stage === 'set-mode') {
return Object.freeze({ rolledBack: false, note: 'Auth mode was never changed.' });
}
return Object.freeze({
rolledBack: false,
note: 'Gateway auth was switched to token mode. To undo it deliberately, run '
+ '`openclaw config set gateway.auth.mode none` — that returns the gateway to accepting '
+ 'unauthenticated connections.',
});
},
});

export const repairCatalog = Object.freeze({
'gateway-not-running': gatewayNotRunning,
'auto-update-enabled-warning': autoUpdateEnabled,
'gateway-loopback-no-auth': gatewayLoopbackNoAuth,
});
5 changes: 3 additions & 2 deletions cli/tui/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,9 @@ export function App(props: AppProps) {

const statusLine = () => {
const state = current()
const rev = state.revision ? ` · revision ${state.revision}` : ""
const full = `🦞 ClawFix v${cliPackage.version}${rev} · ${state.status}`
// The bridge's status already begins with "Revision <id> · N findings", so adding the
// revision here printed it twice and pushed the finding count off the end of the line.
const full = `🦞 ClawFix v${cliPackage.version} · ${state.status}`
const budget = dims().width - 4
if (budget <= 0) return ""
if (full.length <= budget) return full
Expand Down
13 changes: 9 additions & 4 deletions cli/tui/src/components/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ export function Sidebar(props: SidebarProps) {
}
return [...bySeverity.entries()].sort((a, b) => severityRank(a[0]) - severityRank(b[0]))
}
// Carry each finding's real position before sorting. `fix <#>` and `explain <#>` index the
// unsorted findings list, so a sidebar that renumbered its own severity-sorted view sent
// users to the wrong finding — reading "3. Auto-update enabled" and typing `fix 3` hit an
// advisory finding and answered "this finding has no reviewed automatic repair".
const top = () =>
[...(props.findings || [])]
.sort((a, b) => severityRank(a.severity) - severityRank(b.severity))
(props.findings || [])
.map((finding, index) => ({ finding, position: index + 1 }))
.sort((a, b) => severityRank(a.finding.severity) - severityRank(b.finding.severity))
.slice(0, 4)
const aiLabel = () =>
props.aiMode === "remote" ? "Remote" : props.aiMode === "remote-pending" ? "Remote (pending)" : "Local only"
Expand Down Expand Up @@ -69,8 +74,8 @@ export function Sidebar(props: SidebarProps) {
<Spacer />

{top().length > 0 && <text fg={theme.brand}>Top issues</text>}
{top().map((f, i) => (
<text fg={severityColor(f.severity)}>{`${i + 1}. ${f.title}`}</text>
{top().map((entry) => (
<text fg={severityColor(entry.finding.severity)}>{`${entry.position}. ${entry.finding.title}`}</text>
))}
{top().length > 0 && <Spacer />}

Expand Down
73 changes: 73 additions & 0 deletions cli/tui/test/sidebar-numbering.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { afterEach, describe, expect, test } from "bun:test"

import { testRender } from "@opentui/solid"

import { App, createFakeSession } from "../src/app"
import type { TuiSessionView } from "../src/session-bridge"

const renderers: Array<{ destroy(): void }> = []
afterEach(() => { while (renderers.length) renderers.pop()?.destroy() })

/** Findings in the order the session produces them — which is what `fix <#>` indexes. */
const FINDINGS = [
{ id: "f1", title: "Gateway is not running", severity: "critical", repairable: true, repairId: "gateway-not-running" },
{ id: "f2", title: "Auto-update enabled", severity: "medium", repairable: true, repairId: "auto-update-enabled-warning" },
{ id: "f3", title: "Reverse proxy headers are not trusted", severity: "medium", repairable: false, repairId: null },
{ id: "f4", title: "Gateway auth missing on loopback", severity: "critical", repairable: true, repairId: "gateway-loopback-no-auth" },
]

async function frame(partial: Partial<TuiSessionView>, width = 110, height = 34) {
const view = Object.freeze({ ...createFakeSession(), ...partial }) as TuiSessionView
const setup = await testRender(() => <App session={view} />, { width, height })
renderers.push(setup.renderer)
await setup.renderOnce()
return setup.captureCharFrame()
}

describe("sidebar numbering matches the fix/explain selectors", () => {
test("each listed issue carries its position in the findings list, not its sorted rank", async () => {
const out = await frame({ findings: FINDINGS as any, revision: "rev-1" })

// Sorted by severity for prominence: the two criticals lead. But the numbers must stay
// the finding's real index, because `fix 4` has to reach "Gateway auth missing".
// The sidebar wraps, so match the numbered prefix rather than the full title.
expect(out).toContain("1. Gateway is not")
expect(out).toContain("4. Gateway auth")
expect(out).toContain("2. Auto-update enabled")
expect(out).toContain("3. Reverse proxy")

// The old bug: severity-sorted renumbering labelled the auth finding "2".
expect(out).not.toContain("2. Gateway auth")
expect(out).not.toContain("3. Auto-update enabled")
})

test("severity ordering still puts criticals first", async () => {
const out = await frame({ findings: FINDINGS as any, revision: "rev-1" })
const rows = out.split("\n")
const idx = (needle: string) => rows.findIndex((r) => r.includes(needle))
expect(idx("4. Gateway auth")).toBeLessThan(idx("2. Auto-update enabled"))
})
})

describe("status line", () => {
test("does not print the revision twice", async () => {
const out = await frame({
findings: FINDINGS as any,
revision: "aee53dec-5ed0-46e0-a300-c44bf5b0aa28",
status: "Revision aee53dec-5ed0-46e0-a300-c44bf5b0aa28 · 4 findings",
})
const status = out.split("\n").find((r) => r.includes("ClawFix v")) || ""
const occurrences = (status.match(/aee53dec/g) || []).length
expect(occurrences).toBe(1)
})

test("keeps the finding count visible instead of truncating it away", async () => {
const out = await frame({
findings: FINDINGS as any,
revision: "aee53dec-5ed0-46e0-a300-c44bf5b0aa28",
status: "Revision aee53dec-5ed0-46e0-a300-c44bf5b0aa28 · 4 findings",
})
const status = out.split("\n").find((r) => r.includes("ClawFix v")) || ""
expect(status).toContain("4 findings")
})
})
Loading