Skip to content

Picker copy follow-ups: raw rows name the listener, openclaw names the restart, claude-desktop reassurance - #647

Open
philcunliffe wants to merge 2 commits into
masterfrom
fix/issue-637
Open

Picker copy follow-ups: raw rows name the listener, openclaw names the restart, claude-desktop reassurance#647
philcunliffe wants to merge 2 commits into
masterfrom
fix/issue-637

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Three picker rows changed the machine without saying so in the one place the user reads before choosing. Each claim below was verified against the code before it was written.

raw-anthropic / raw-openai (hypaware-core/plugins-workspace/ai-gateway/hypaware.plugin.json)

  • Before: For apps you manually point at HypAware.
  • After: For apps you manually point at HypAware; starts a local gateway listener.

Evidence: both rows set compose.requires_gateway: true (ai-gateway/hypaware.plugin.json:31,40). composePickerConfig turns that into a @hypaware/ai-gateway plugin instance (src/core/cli/walkthrough.js:774,798-804), whose source binds an HTTP server (ai-gateway/src/source.js:189 -> ai-gateway/src/proxy.js:49,58,82). Host/port is 127.0.0.1:18521 by default (ai-gateway/src/config.js:8). The port is deliberately not in the copy: the picker writes no listen, so a taken port silently falls back to an ephemeral one (config.js:12, walkthrough.js:795-797, LLP 0114), and naming 18521 would have been a claim that is not always true. 127.0.0.1 is always true, hence "local".

This is the same class of side effect the otel row already names ("starts a local receiver"), and these two rows compose no adapter that could disclose it anywhere else.

openclaw (hypaware-core/plugins-workspace/openclaw/hypaware.plugin.json)

  • Before: Live traffic plus local session history; attach rewrites OpenClaw's gateway config.
  • After: Live traffic plus local session history; attach rewrites OpenClaw's gateway config, then you run 'openclaw gateway restart'.

Evidence: openclaw/src/attach.js:54 defines RESTART_COMMAND = 'openclaw gateway restart' and :62-63 the instruction attach prints, documented as required because a running OpenClaw gateway keeps routing at the old baseUrl until restarted (verified 2026.3.13, LLP 0167#verify-results item 4). attach prints it only after the config has been rewritten, so the picker row is the only place the user can learn it before choosing.

The manifest string and RESTART_COMMAND were only linked by eyeball; test/plugins/picker-disclosure.test.js now imports RESTART_COMMAND from attach.js (now exported) and asserts the summary contains it, so a future rename of the command fails this test instead of silently leaving the manifest pointing at a command that no longer exists.

claude-desktop (hypaware-core/plugins-workspace/claude-desktop/hypaware.plugin.json)

  • Before: Requires Claude sign-in and admin approval.
  • After: Requires Claude sign-in and admin approval; starts a local gateway listener; setup asks before changing anything.

Evidence: the row is needs_setup: true with configure_command: "claude-desktop install" (claude-desktop/hypaware.plugin.json:29-30), the wizard runs that command through the configure phase (src/core/cli/wizard/configure.js:81), and runInstall gates every applying step behind an explanation and a confirm that defaults to no before it touches the credential, helper, residue, or plist (claude-desktop/src/install.js:145-158, @ref LLP 0139#informed-consent). The reassurance is load-bearing next to "admin approval", which reads as a sudo ambush without it.

The listener clause is separate from the setup step: the row's compose also sets requires_gateway: true with a bound gateway_upstream (claude-desktop/hypaware.plugin.json:32-38), composed into the daemon config as soon as the row is picked, independent of whether claude-desktop install has run. It is ordered before the setup clause so it does not read as something setup causes.

claude (hypaware-core/plugins-workspace/claude/hypaware.plugin.json)

  • Before: CLI and SDK conversations, including OpenClaw's claude-cli backend; attaches Claude Code and installs helper skills.
  • After: CLI and SDK conversations, including OpenClaw's claude-cli backend; attaches Claude Code, starts a local gateway listener, and installs helper skills.

Evidence: same as claude-desktop above, this row's compose sets requires_gateway: true with a bound gateway_upstream (claude/hypaware.plugin.json:43-44), so picking it alone binds a real listener. This row was added to the PR after an earlier draft of this description incorrectly named hermes as the row this issue left out of scope; see below for why that was wrong and claude/claude-desktop are the actual outliers.

Why hermes, not claude/claude-desktop, is genuinely out of scope

An earlier version of this PR (and its description) said hermes was the remaining row that sets requires_gateway without naming a listener, and left it alone as a scoping note. That was backwards.

hermes/hypaware.plugin.json:27-29 sets requires_gateway: true with no gateway_upstream. Picked alone, composePickerConfig produces a @hypaware/ai-gateway plugin instance with upstreams: [] (pinned at test/core/compose-picker-config.test.js:148-160, comment there notes compileUpstreams accepts the empty list at config-compile time). But ai-gateway/src/proxy.js:52-54 throws 'ai-gateway: at least one upstream must be configured before start' when the compiled upstream list used to start the listener is empty, and hermes registers no upstream preset of its own (hermes/src/index.js has no registerUpstreamPreset call). So a hermes-only install does not compose a working listener at all; today it throws at gateway source start. Omitting a listener clause from hermes's summary is therefore arguably correct, not an oversight, and is left alone here.

claude and claude-desktop, by contrast, both set requires_gateway with a gateway_upstream, so both really do produce a bound listener when picked, and neither summary said so. claude-desktop was the sharper miss: this PR's original commit edited that exact summary string to add the "asks before changing anything" reassurance while declining to add the listener clause it added two rows over on raw-anthropic/raw-openai. Both are now fixed above.

Separately: the hermes-alone throw looks like a pre-existing bug (a picker row that composes a requires_gateway config with no way to reach a working upstream). It reproduces by code inspection and by direct invocation of startProxy/mergeUpstreams with the exact config compose-picker-config.test.js pins for hermes-alone. It predates this PR, is unrelated to picker copy, and is not fixed here; filing separately.

Tests

Extended test/plugins/picker-disclosure.test.js with six pins (openclaw restart command sourced from attach.js's RESTART_COMMAND rather than a copied literal, raw-anthropic listener, raw-openai listener, claude-desktop reassurance, claude listener, claude-desktop listener), each asserting the substance rather than exact wording so the copy stays re-tunable but the admission cannot vanish again. Each pin carries a comment citing the code it is derived from. Reverting each new manifest clause locally was confirmed to fail its corresponding test before restoring it.

Checked and unchanged because they do not assert this copy: test/plugins/openclaw-manifest.test.js (matches /live/i, /session history/i, /claude-cli/), test/core/config.test.js (synthetic manifest fixtures), test/core/walkthrough-prompt.test.js (unanchored prefix match on the Anthropic row, still true after the suffix). No assertion was loosened.

node scripts/run-tests.js: 3580 pass, 0 fail, 1 skipped. npx tsc -p tsconfig.json --noEmit: clean.

No LLP text touched. LLP 0139 is Accepted and records the consent gate this copy now surfaces; it needed no change.

Fixes #637

🤖 Generated with Claude Code

test and others added 2 commits August 5, 2026 23:46
…e restart, claude-desktop reassurance

Three picker rows changed the machine without saying so in the one place
the user reads before choosing.

- raw-anthropic / raw-openai set compose.requires_gateway, which composes
  @hypaware/ai-gateway and binds an HTTP listener on 127.0.0.1 (default
  127.0.0.1:18521). The otel row already names its receiver; these two
  named nothing, and they carry no adapter to disclose it elsewhere.
- openclaw's summary named the config rewrite but not the manual
  'openclaw gateway restart' that makes it take effect. attach.js prints
  that only after the config is already rewritten.
- claude-desktop lost its "asks before changing anything" reassurance in
  the copy shortening. Its machine change is behind needs_setup +
  configure_command 'claude-desktop install', which explains and asks,
  defaulting to no (LLP 0139#informed-consent), so the reassurance was
  accurate.

Each new clause is pinned in test/plugins/picker-disclosure.test.js by
substance rather than exact wording, so a future shortening pass fails
instead of silently dropping the disclosure again.

Fixes #637

Co-Authored-By: Claude <noreply@anthropic.com>
…tener, restart command pinned to source

Review of #647 found two LOW issues:

- The PR named hermes as the remaining requires_gateway row left out of
  scope, which is backwards. hermes sets requires_gateway with no
  gateway_upstream, so picked alone it composes an empty upstream list
  (test/core/compose-picker-config.test.js:148-160) and
  ai-gateway/src/proxy.js:52-54 throws before a listener ever binds -
  omitting the clause there is arguably correct. claude and
  claude-desktop both set requires_gateway *with* a gateway_upstream, so
  both really do bind a listener, and neither summary said so.
  claude-desktop is the sharper miss: this PR's own commit added a
  reassurance clause to that exact string while declining to add the
  listener clause it added two rows over on raw-anthropic/raw-openai.
  Both are now fixed, and the PR body's completeness note is corrected
  to name the real outliers and explain why hermes is different.

- test/plugins/picker-disclosure.test.js only asserted the openclaw
  summary against a copy of 'openclaw gateway restart', so a rename of
  attach.js's RESTART_COMMAND would silently leave the manifest
  pointing at a command that no longer exists. RESTART_COMMAND is now
  exported from attach.js and the test asserts the summary contains it.

Confirmed (not fixed here, pre-existing and unrelated): a hermes-only
install throws 'ai-gateway: at least one upstream must be configured
before start' at gateway source start, reproduced directly against
proxy.js/source.js with the exact config compose-picker-config.test.js
pins for hermes alone.

Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review round 1 - bfc8482

All three disclosure claims verified TRUE against the code, not just the manifests. Two LOW findings, both fixed in d49f60b.

  • raw rows / "starts a local gateway listener": requires_gateway flows through composePickerConfig (walkthrough.js:774,792-802) to a real server.listen (ai-gateway/src/proxy.js:49-83). Accurate.
  • openclaw / "then you run 'openclaw gateway restart'": matches RESTART_COMMAND at attach.js character for character, and a non-refused attach always rewrites, so a restart is always warranted. Refusals correctly suppress it.
  • claude-desktop / "setup asks before changing anything": the consent gate fires before every mutating step and defaults to no (consent.js:135), with cancel/EOF/absent-stdin all returning false. Nothing mutates above the gate.

The omitted port was the right call, confirmed: the picker writes no listen per LLP 0114, so a taken port falls back to an ephemeral bind, and a row promising 18521 would be false whenever that fires.

Findings

1. LOW - the PR named the wrong row as the out-of-scope outlier, and was internally inconsistent. It cited hermes, but hermes sets requires_gateway with no gateway_upstream, so picked alone it composes { upstreams: [] } and starts no listener at all. The real outliers are claude and claude-desktop, both of which set requires_gateway with an upstream and so do bind a listener, and neither said so. claude-desktop was the sharp one, because this very commit edited that string to add a reassurance while declining to add the listener clause it added two rows over.

Fixed by folding both in:

  • claude: ...attaches Claude Code, starts a local gateway listener, and installs helper skills.
  • claude-desktop: Requires Claude sign-in and admin approval; starts a local gateway listener; setup asks before changing anything.

The PR body has been corrected to name the real outliers and explain why hermes genuinely differs.

2. LOW - the openclaw test did not pin manifest-to-code agreement. It asserted a literal /openclaw gateway restart/i against the manifest, so renaming RESTART_COMMAND would leave the manifest quoting a wrong command with the test still green. Fixed: RESTART_COMMAND is now exported and the test asserts the summary contains it.

Pins are real, not non-emptiness

Verified by reverting each clause and confirming the matching test fails, then restoring: the claude and claude-desktop listener clauses each fail their own test, and drifting RESTART_COMMAND to 'openclaw gw restart' fails the openclaw test. No existing copy-asserting test was loosened; openclaw-manifest.test.js, config.test.js and walkthrough-prompt.test.js are untouched and pass.

npm test 3581 tests / 3580 pass / 0 fail / 1 skip, tsc clean, all 22 manifests valid JSON, zero non-ASCII in the changed files.

Separately filed: the hermes-alone gateway throw the review surfaced is real and reproduces; it is pre-existing and unrelated to this PR, so it was deliberately not fixed here.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review round 2 - d49f60b - CLEAN

Both round-1 findings fixed, and the two new disclosure claims verified TRUE rather than plausible.

1. claude / claude-desktop listener - FIXED and accurate. Both rows set compose.requires_gateway with a gateway_upstream (claude :43-44, claude-desktop :32-40), traced through composePickerConfig to startProxy's unconditional server.listen. Same mechanism already verified for otel and the raw rows in round 1. The contrast holds too: hermes sets requires_gateway with no upstream, so a listener claim there would be false, and its summary correctly omits it. These are accurate disclosures, not upgrades to false ones.

2. RESTART_COMMAND pinned to source - FIXED. Exported from attach.js:54 (value unchanged, only the export and a comment added) and the test now asserts the manifest contains the constant, so the two cannot drift.

Pins independently re-verified

Reverted each change one at a time and confirmed exactly one test fails each time, restoring between runs: claude clause reverted fails the claude-listener test, claude-desktop clause fails its own, and drifting RESTART_COMMAND fails the openclaw test. Baseline 10/10 pass, each revert 9 pass / 1 fail, restored 10/10.

No collateral, and the set is now complete

git diff origin/master HEAD -- test/ touches only picker-disclosure.test.js and is purely additive, zero removed lines. openclaw-manifest.test.js, config.test.js and walkthrough-prompt.test.js are untouched.

A fresh pass over all 8 picker rows across all 7 manifests with a picker key found no remaining undisclosed machine-changing side effect: claude, claude-desktop, codex, openclaw, both raw rows and otel all name theirs, and hermes correctly claims none because it has none.

Copy

The clause order on claude-desktop is load-bearing and correct: the listener clause precedes "setup asks before changing anything", because the listener starts when the row is picked, independent of the needs_setup consent gate that only guards the plist install. Reversing them would make the reassurance read as if it covered the listener too. claude's three-item list follows a sensible causal order.

npm test 3580 pass / 0 fail / 1 skip, tsc clean, all manifests valid JSON, no em dashes or smart quotes. No new findings.

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Picker copy follow-ups: raw rows name the listener, openclaw names the restart, claude-desktop reassurance

1 participant