OpenClaw client adapter plugin (LLP 0109) by platypii · Pull Request #317 · hyparam/hypaware

platypii · 2026-07-15T20:32:08Z

Adds @hypaware/openclaw, a client adapter plugin for OpenClaw, following the Claude/Codex pattern.

Attach injects a dedicated hypaware model provider into ~/.openclaw/openclaw.json (Anthropic Messages protocol, gateway base URL) and repoints agents.defaults.model.primary, recording a self-describing undo record in the provider's headers map. OpenClaw strictly validates its config and rejects unknown root keys, so the usual top-level JSON marker is not an option; see LLP 0109 for the full rationale.
Core gains a third attach-probe format, json_path (nested marker path + record path), wired through the status probe and the single disk-driven detach undo. Claude/codex paths untouched.
New openclaw exchange projector matches on the injected x-hypaware-client header (priority above the Claude projector on the shared /v1/messages path); the gateway now strips x-hypaware-* request headers before proxying upstream.
cli_bundled_plugins_activated smoke: fixed the drifted plugins_skipped count (was stale since ai-gateway-graph joined the allowlist) and extended it to activate OpenClaw and dry-run its attach against a seeded OPENCLAW_HOME.

V1 limitations (documented in LLP 0109): Anthropic primaries only, auth via ${ANTHROPIC_API_KEY}, no skills shipped yet.

Verification: npm test (2308 tests) and typecheck clean; cli_bundled_plugins_activated, gateway_codex_capture, client_attach_idempotent smokes pass. gateway_claude_capture and claude_attach_detach fail identically on a clean master checkout (pre-existing, unrelated).

…109)

philcunliffe · 2026-07-15T22:00:03Z

Dual-agent review — `request_changes`

Verdict: request_changes
Risk class: medium
Auto-merge advisory: 👎 thumbs down — verdict is request_changes; needs human-gated follow-up

Advisory only: no merge was attempted.

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Source	Finding (severity, evidence)	Intersects
Codex	major — reattach same-port noop ignores primary/allowlist drift (settings.js:225)	Risks #1, Direct callers
Codex	minor — LLP 0109 cache-warning documented but unimplemented (settings.js:296)	Risks #4, Config chain
Codex	minor — OpenClaw smoke uses port 0, skips config validation (cli_bundled_plugins_activated.js:65)	Config field chain
Claude	minor — inline import('...') type violates CLAUDE.md (settings.js:365)	Files

Codex review

Fix Validations

cli_bundled_plugins_activated stale plugin counts

Status: correct
Evidence: src/core/runtime/bundled.js:31, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:316, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:326
Assessment: The smoke now accounts for @hypaware/openclaw in the configured activation set and expects plugins_activated=7 / plugins_skipped=5, matching the updated bundled allowlist.

Findings

1) Behavioral Correctness

Severity: major
Confidence: high
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:118, hypaware-core/plugins-workspace/openclaw/src/settings.js:156, hypaware-core/plugins-workspace/openclaw/src/settings.js:166, hypaware-core/plugins-workspace/openclaw/src/settings.js:225, src/core/daemon/status.js:938
Why it matters: A marker-present OpenClaw config can drift so agents.defaults.model.primary no longer points at hypaware/<model> or the allowlist no longer contains it, but a later hyp attach --client openclaw same-port run returns noop and status still reports attached, leaving traffic uncaptured.
Suggested fix: In reattach(), replay or verify the recorded managed.set and managed.appended entries before returning no-op; either restore them on explicit attach or fail with a conflict that tells the user what changed. Add tests for marker-present primary drift and allowlist drift.

2) Contract & Interface Fidelity

Severity: minor
Confidence: high
Evidence: llp/0109-openclaw-client-adapter.decision.md:169, llp/0109-openclaw-client-adapter.decision.md:171, hypaware-core/plugins-workspace/openclaw/src/settings.js:296, hypaware-core/plugins-workspace/openclaw/src/settings.js:299
Why it matters: LLP 0109 says attach warns when stale ~/.openclaw/agents/<id>/agent/models.json cache entries can override config, but attach() only emits the ANTHROPIC_API_KEY warning and never scans or names those cache files.
Suggested fix: Implement the documented cache scan/warning, or amend LLP 0109 before merge if the warning is no longer part of V1. Add a targeted attach test with a stale cached hypaware provider.

9) Test Evidence Quality

Severity: minor
Confidence: medium
Evidence: hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:65, src/core/commands/clients.js:196, hypaware-core/plugins-workspace/openclaw/src/settings.js:310, hypaware-core/plugins-workspace/openclaw/src/settings.js:591, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:232
Why it matters: The added OpenClaw smoke uses a 127.0.0.1:0 gateway endpoint, and OpenClaw dry-run skips readSettings() / prepareAttach() when the derived port is undefined, so the seeded OPENCLAW_HOME config is not actually validated by that smoke.
Suggested fix: Make dry-run validate the config with a sentinel valid port when the endpoint is the placeholder, or give this smoke a concrete configured endpoint and assert the planned provider/model lines.

No Finding

Change Impact / Blast Radius
Concurrency, Ordering & State Safety
Error Handling & Resilience
Security Surface
Resource Lifecycle & Cleanup
Release Safety
Architectural Consistency
Debuggability & Operability

Evidence Bundle

Changed hot paths: hypaware-core/plugins-workspace/openclaw/src/settings.js:112, hypaware-core/plugins-workspace/openclaw/src/settings.js:296, src/core/config/client_detach_disk.js:85, src/core/daemon/status.js:903, hypaware-core/plugins-workspace/openclaw/src/projector.js:47, hypaware-core/plugins-workspace/ai-gateway/src/proxy.js:143
Impacted callers: hypaware-core/plugins-workspace/openclaw/src/index.js:74, src/core/commands/clients.js:196, src/core/commands/clients.js:373, src/core/daemon/status.js:491
Impacted tests: test/plugins/openclaw-settings-attach.test.js:126, test/plugins/openclaw-settings-attach.test.js:137, test/core/client-detach-json-path.test.js:208, test/plugins/openclaw-projector.test.js:70, test/plugins/ai-gateway-proxy-routing.test.js:20, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:232
Unresolved uncertainty: I did not run the suite; the stale OpenClaw cache behavior is based on LLP 0109’s documented OpenClaw behavior, not a live OpenClaw install.

Claude review

Claude review

Inline `import('...')` type annotation in openclaw settings.js

Severity: minor
Confidence: 85
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:365
Why it matters: CLAUDE.md forbids inline import('...') types ("Never use inline import('...') types. Declare type imports at the top of the file with @import"); this is a newly-authored file that already has a top-level @import block, so the inline import('node:fs').Stats is an introduced style violation, not pre-existing.
Suggested fix: Add @import { Stats } from 'node:fs' to the file's top-level @import block and change the annotation to /** @type {Stats | undefined} */.

Reports: /Users/phil/workspace/hypaware/.git/worktrees/tmp.5qokK2Zpfs/dual-review/pr-317

Review heal on adopted PR #317: CLAUDE.md forbids inline import('...') types. Hoist Stats to the existing top-level @import block and reference the bare name, matching the file's own convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-07-15T22:07:02Z

neutral review round 1 (adopt, full-heal) reviewed head dc98d5c. Dual reviewers (Codex + Claude) both completed. Mechanical verdict: request_changes (medium risk), driven by one behavioral-correctness major. No blockers.

Healed by neutral in this round (pushed to openclaw-plugin, now at 94fe72f):

[minor, style] hypaware-core/plugins-workspace/openclaw/src/settings.js:365 inline import('node:fs').Stats type violated the CLAUDE.md ban on inline import('...') types. Hoisted to the top-level @import { Stats } from 'node:fs' block. This is the only design-neutral, repo-convention fix; the rest are yours to decide.

Handed back for your decision (neutral will not rewrite the adapter's design):

[MAJOR, correctness] settings.js:225-227 (reattach): a same-port re-attach returns noop on record.port === opts.port alone, without re-verifying that agents.defaults.model.primary still points at hypaware/<model> or that the allowlist still contains it. If the marker provider entry survives but the repoint drifts (a user edit or an OpenClaw hot-reload revert), traffic goes uncaptured while status still probes as attached (probe keys off marker-path existence only). Genuine silent "attached-but-not-capturing" gap for a capture adapter. Requires external drift to trigger (attach never causes it), so it is a robustness gap, not a crash/data-loss blocker. Suggested fix: replay/verify the recorded set+appended on same-port re-attach (or fail with a conflict), plus drift tests.
[minor, doc drift] settings.js:296 (attach): LLP 0109 Known-v1-limitations specifies attach warns when a stale ~/.openclaw/agents/<id>/agent/models.json cache entry could override config; attach() emits only the ANTHROPIC_API_KEY warning and never scans/names those cache files. Implement the warning or descope it in the doc before merge.
[minor, test] hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:65: the new OpenClaw smoke uses 127.0.0.1:0; the dry-run path skips readSettings()/prepareAttach() when the derived port is undefined, so the seeded OPENCLAW_HOME config is never actually validated by that smoke. Give it a concrete port to make the smoke load-bearing.

Verified sound (not findings): the json_path core probe/detach undo logic in client_detach_disk.js (strict-equality restore, external-override warn, external-delete skip, appended removal, unconditional marker_path deletion, deepest-first pruning of recorded created_parents, malformed-marker non-destructive failure, mtime-gated atomic writes); projector priority 110 > Claude's 100 correctly winning the shared /v1/messages path; matching on the original unstripped headers while the proxy strips x-hypaware-* only on the upstream copy; LLP 0109 present and correctly @ref'd; root-anchored .js type-import specifiers.

Full dual-review report: see the summary comment above. Reply here or push to openclaw-plugin and neutral will re-engage on its next tick.

…oad-bearing (LLP 0109) Fixes three review findings on the OpenClaw client adapter plugin. MAJOR: a same-port re-attach no longer trusts marker-path existence alone. reattach() now replays the recorded managed set + appended mutations against the live config: if the primary drifted off hypaware/<model> or the allowlist lost the managed id (a user edit or an OpenClaw hot-reload revert), the drift is re-applied so capture is restored instead of a silent noop while the core probe still reports attached. The ORIGINAL managed undo record is preserved, so detach still restores the user's own pre-attach values. If the marker parent object was removed wholesale (re-point would silently no-op), attach refuses loudly (DRIFT_CONFLICT) rather than leave capture believed-on. MINOR: attach() now scans for OpenClaw's per-agent resolved-provider caches (<home>/agents/<id>/agent/models.json) and warns naming the exact files that could shadow the injected hypaware provider (LLP 0109 known-v1-limitations), without editing agent state. MINOR: the cli_bundled_plugins_activated smoke gave the ai-gateway listen a concrete port (was 127.0.0.1:0). A :0 listen resolves the derived dry-run endpoint to port 0, which the adapter treats as the unstarted placeholder and skips readSettings()/prepareAttach(), so the seeded OPENCLAW_HOME config was never validated. The port is never bound in a CLI dispatch, so it cannot collide across parallel runs. Adds regression tests (LLP 0002): re-attach drift-heal at prepareAttach and attach() I/O level (asserting the core json_path probe reflects real capture), DRIFT_CONFLICT refusal, and the stale-cache warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-07-15T22:55:41Z

Dual-agent review — `request_changes`

Verdict: request_changes
Risk class: medium
Auto-merge advisory: 👎 thumbs down — verdict is request_changes; needs human-gated follow-up

Advisory only: no merge was attempted. Round 2 re-review at head ab01bcb after the round-1 fixes.

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Source	Finding (severity, evidence)	Intersects
Codex	major — same-port reattach does not heal provider baseUrl/client-header/models drift (settings.js:200-245)	Risks (attached-but-not-capturing residual), Targets (reattach/reapplyManaged)

Codex review

Fix Validations

Bundled smoke skipped-count drift

Status: correct
Evidence: src/core/runtime/bundled.js:31, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:60, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:324, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:333
Assessment: The smoke now selects seven allowlisted plugins including @hypaware/openclaw, and the expected skipped count of five matches the remaining allowlisted plugins omitted by the staged config.

Findings

1) Behavioral Correctness

Severity: major
Confidence: high
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:200, hypaware-core/plugins-workspace/openclaw/src/settings.js:234, hypaware-core/plugins-workspace/openclaw/src/settings.js:236, hypaware-core/plugins-workspace/openclaw/src/settings.js:245, hypaware-core/plugins-workspace/openclaw/src/projector.js:51, src/core/daemon/status.js:937
Why it matters: Same-port reattach can report noop while the managed provider no longer routes or attributes traffic correctly.
Suggested fix: In reattach(), re-derive and restore the critical provider fields on every run, not only on port changes: baseUrl, api, apiKey, headers[x-hypaware-client], marker header, and models; add tests for same-port drift of baseUrl, client header, and provider model list.

The current reattach path only replays recorded set and appended mutations, then returns immediately when record.port === opts.port. If the marker record survives but baseUrl is changed away from the gateway, or x-hypaware-client is removed, status still probes attached by marker-path existence and the OpenClaw projector no longer matches.

No Finding

Contract & Interface Fidelity
Change Impact / Blast Radius
Concurrency, Ordering & State Safety
Error Handling & Resilience
Security Surface
Resource Lifecycle & Cleanup
Release Safety
Test Evidence Quality, aside from the coverage gap tied to the finding above
Architectural Consistency
Debuggability & Operability

Evidence Bundle

Changed hot paths: OpenClaw attach/reattach writer, json_path status probe, json_path disk detach, OpenClaw exchange projector, AI gateway header forwarding, bundled-plugin activation.
Impacted callers: src/core/commands/clients.js:93, src/core/commands/clients.js:105, src/core/config/action_attach.js:75, src/core/config/action_attach.js:239, hypaware-core/plugins-workspace/ai-gateway/src/proxy.js:143, hypaware-core/plugins-workspace/ai-gateway/src/proxy.js:144, hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:521.
Impacted tests: test/core/client-detach-json-path.test.js:132, test/plugins/openclaw-settings-attach.test.js:185, test/plugins/openclaw-projector.test.js:71, test/plugins/ai-gateway-proxy-routing.test.js:13, test/plugins/openclaw-config.test.js:10.
Unresolved uncertainty: The local worktree appeared to be base, not PR head, so new-file line evidence is from the provided diff; existing caller traces came from the worktree.

Claude review

Claude review

No issues found.

Reviewed across guidance-compliance, shallow-bug, and contract/callers facets plus direct verification of the round-2 fix code:

reapplyManaged heals drift by writing entry.value (never entry.prev) into config, leaving the undo record's prev fields intact; detach still restores pre-attach values.
DRIFT_CONFLICT guard fires only when a drifted primary's parent object was removed wholesale (no false positive on the no-drift early-continue path).
Heal preserves atomic + mtime-gated + no-symlink writes (heal is a pure transform in prepareAttach; attach() still routes through writeAtomic/readSettings).
The 5 drift/heal regression tests are genuinely fail-before/pass-after (verified by neutralizing reapplyManaged: all 5 fail).
The smoke port 4317 is load-bearing (verified: breaking the seeded primary to non-Anthropic makes the smoke FAIL) and cannot collide across runs (port never bound in CLI dispatch).
findStaleModelCaches over-warns on cache existence rather than baseUrl content, but is non-fatal stderr and matches its documented behavior (below reporting threshold).
Core detach json_path reverses all four managed sub-records; probe/manifest/writer agree on marker_path + marker_record.
CLAUDE.md compliant: no semicolons, no em dashes, no inline import(), root-anchored .js type-import specifiers, correct @ref form.

Reports: /Users/phil/workspace/hypaware/.git/worktrees/tmp.kiP3zFUNpt/dual-review/pr-317

…repoint/allowlist (LLP 0109) Round-1 healed only the recorded config-level mutations (primary repoint + allowlist join). A re-attach still left the injected provider entry's own fields un-reconciled, so if the marker/provider record survived but `baseUrl` was blanked or repointed, the `x-hypaware-client` header was dropped, or `api`/`apiKey`/`models` drifted, the core json_path probe kept reporting ATTACHED (it keys off marker-path existence alone) while traffic routed nowhere or was unattributed - the same silent "attached-but-not-capturing" class as the round-1 MAJOR, different fields. Re-attach now re-derives the complete provider entry from the same buildProvider() first attach uses and reconciles the live config to it (key-order-independent compare via canonicalJson, so an untouched entry stays a no-op). The port-change path folds into the same reconcile. The ORIGINAL managed undo record rides through verbatim, so detach still restores the user's pre-attach originals; DRIFT_CONFLICT still guards a wholesale-removed parent; writes stay atomic/mtime-gated/symlink-refusing (heal is a pure transform in prepareAttach, attach() writes only when changed). Factored: attachRecord() (shared marker-envelope builder) and reconcileProvider(); reattachModel() derives the model from the authoritative recorded repoint value rather than the drift-prone provider models list. Removed an unused JsonObject @import. Tests: same-port-reattach drift cases for baseUrl (blanked + repointed), removed x-hypaware-client header, and drifted models list, plus an attach() I/O + probeClientAttachFromDescriptor case proving real capture after heal. Each fails on pre-fix code (verified by neutralizing the new reconcile) and passes after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-07-15T23:24:53Z

Dual-agent review — `request_changes`

Verdict: request_changes
Risk class: medium
Auto-merge advisory: 👎 thumbs down — verdict is request_changes; needs human-gated follow-up

Advisory only: no merge was attempted. Note: verdict is driven by the medium blast radius plus minor findings; no blocker or major was raised, and the reworked reattach() path (this round's focus) is clean on all 7 scrutinized risks.

Risk capstone

Cross-reference (reviewer findings vs surface)

Finding	Reviewer	Severity	In this round's focus (reattach)?
Projector empty tool-use input overwrite	Codex + Claude	minor	No (projector.js)
Projector session-id fallback crash on missing content	Claude	minor	No (projector.js)
Dry-run `:0` endpoint skips config read/validate	Codex + Claude	minor	No (attach() dry-run)
Reattach MALFORMED_MARKER refusal untested	Claude	minor	Adjacent (test gap)
attach()-level true-no-op untested	Claude	minor	Adjacent (test gap)
LLP 0109 still Draft on merge	Claude	nit	No (doc hygiene)
Inherited em dash at client_detach_disk.js:27	Claude	nit	No (reflowed prose)
api/apiKey field-heal untested	Claude	nit	Adjacent (covered by wholesale compare)

No reviewer raised a blocker or major. Every substantive finding lands OUTSIDE
the reworked reattach code; the reattach rework itself is unanimously clean.

Codex review

Fix Validations

Bundled-plugin smoke skipped count drift

Status: correct
Evidence: src/core/runtime/bundled.js:31, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:327, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:337
Assessment: The allowlist includes @hypaware/openclaw, and the smoke expectations now match 7 activated plugins and 5 skipped plugins.

Re-attach heals marker-present capture drift

Status: correct
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:331, hypaware-core/plugins-workspace/openclaw/src/settings.js:385, src/core/daemon/status.js:938, test/plugins/openclaw-settings-attach.test.js:192
Assessment: The core probe is marker-path based, so it cannot detect a drifted primary/provider. The PR correctly adds reattach healing for both recorded config mutations and the full provider block.

Findings

1) Behavioral Correctness

Severity: minor
Confidence: high
Evidence: src/core/commands/clients.js:196, hypaware-core/plugins-workspace/openclaw/src/settings.js:471, hypaware-core/plugins-workspace/openclaw/src/settings.js:475, test/plugins/openclaw-settings-attach.test.js:724
Why it matters: hyp attach --client openclaw --dry-run can report success without reading or validating openclaw.json when the gateway endpoint falls back to http://127.0.0.1:0.
Suggested fix: In dry-run, still read settings and validate with a harmless valid placeholder port, or fail explicitly when no usable endpoint is known. Add a dry-run test with endpoint http://127.0.0.1:0 and a non-Anthropic primary.

2) Contract & Interface Fidelity

Severity: minor
Confidence: medium
Evidence: hypaware-core/plugins-workspace/openclaw/src/projector.js:250, hypaware-core/plugins-workspace/openclaw/src/projector.js:266, hypaware-core/plugins-workspace/openclaw/src/projector.js:310, test/plugins/openclaw-projector.test.js:161
Why it matters: Streamed tool-use blocks with no input_json_delta overwrite the block’s existing input with '', corrupting valid empty-input tool calls in projected telemetry.
Suggested fix: Only overwrite block.input when accumulated partial JSON is non-empty; otherwise preserve the start block’s input, or default tool-use input to {}. Add SSE tool-use tests for empty and non-empty inputs.

No Finding

1. Change Impact / Blast Radius
1. Concurrency, Ordering & State Safety
1. Error Handling & Resilience
1. Security Surface
1. Resource Lifecycle & Cleanup
1. Release Safety
1. Test Evidence Quality
1. Architectural Consistency
1. Debuggability & Operability

Evidence Bundle

Changed hot paths: OpenClaw attach/reattach writer, OpenClaw exchange projector, json_path status probe, json_path disk detach, ai-gateway upstream header forwarding, bundled plugin activation.
Impacted callers: src/core/commands/clients.js:297, src/core/config/action_attach.js:161, src/core/commands/clients.js:373, src/core/daemon/status.js:490, hypaware-core/plugins-workspace/ai-gateway/src/proxy.js:143
Impacted tests: test/core/client-detach-json-path.test.js:208, test/plugins/openclaw-settings-attach.test.js:192, test/plugins/openclaw-projector.test.js:161, test/plugins/ai-gateway-proxy-routing.test.js:20, hypaware-core/smoke/flows/cli_bundled_plugins_activated.js:243
Unresolved uncertainty: I did not run the test suite. I did not verify OpenClaw’s current streamed empty-tool-input event shape against upstream docs during this bounded review.

Claude review

Claude review

Focus of this round: the reworked reattach() path in
hypaware-core/plugins-workspace/openclaw/src/settings.js (single-source
buildProvider/attachRecord, reconcileProvider wholesale heal,
reattachModel). Three parallel reviewers plus a targeted 7-risk deep dive.

Reattach rework verdict: SOUND on all seven risks — no blocker/major/minor
in the new code. Both first-attach and reattach route the marker through the
same attachRecord()/buildProvider() builders (settings.js:183/189 vs :267),
so the envelope cannot diverge; wholesale provider replacement is justified
because the models.providers.hypaware entry is 100% hypaware-owned (attach
creates it via added:[PROVIDER_PATH], core detach removes it wholesale, and
OpenClaw caches resolved providers in a separate file, not this entry);
canonicalJson compare is a true structural equality with a byte-identical
marker round-trip on an untouched same-port reattach (proven no-op); the parsed
managed block rides through verbatim so set[*].prev still names the user's
originals for detach.

The findings below are all OUTSIDE the reattach rework and are minor/nit.

Projector overwrites tool-use input with empty string on streamed empty-input tool calls

Severity: minor
Confidence: 85
Evidence: hypaware-core/plugins-workspace/openclaw/src/projector.js:250,266,310
Why it matters: A streamed tool_use block with no input_json_delta overwrites the block's input with '', corrupting a valid empty-input tool call in projected telemetry.
Suggested fix: Only overwrite block.input when accumulated partial JSON is non-empty; else preserve the start block's input (default {}). Add SSE tool-use tests for empty and non-empty inputs.

Projector session-id fallback throws on a message with no `content` key

Severity: minor
Confidence: 80
Evidence: hypaware-core/plugins-workspace/openclaw/src/projector.js:474-479
Why it matters: hashShort(JSON.stringify(messages[0].content)) → JSON.stringify(undefined) returns the value undefined, and sha256Hex(undefined) throws, dropping the exchange. Only reachable with no system prompt AND absent messages[0].content (unlikely in real traffic), but it crashes rather than degrading in an otherwise defensive file.
Suggested fix: Guard the fallback, e.g. hashShort(JSON.stringify(messages[0].content) ?? exchangeId).

Dry-run can report success without reading/validating openclaw.json when endpoint is 127.0.0.1:0

Severity: minor
Confidence: 80
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:471-481
Why it matters: When safePort yields undefined (placeholder :0 endpoint), the dry-run skips the pure transform, so a refusal (e.g. non-Anthropic primary) is not surfaced in the plan.
Suggested fix: In dry-run, still read+validate with a placeholder valid port, or explicitly report "no usable endpoint". Add a :0-endpoint dry-run test.

Reattach-path MALFORMED_MARKER refusal is untested

Severity: minor
Confidence: 85
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:226-233 (no test in test/plugins/openclaw-settings-attach.test.js)
Why it matters: The distinct attach-side refusal (provider present, marker header missing/unparseable) has no coverage; the detach-side MALFORMED_MARKER is tested but does not exercise this branch.
Suggested fix: Seed a provider with a garbage/absent marker header; assert OpenclawSettingsError code MALFORMED_MARKER.

attach()-level true-no-op (skip write, mtime unchanged) is untested

Severity: minor
Confidence: 80
Evidence: hypaware-core/plugins-workspace/openclaw/src/settings.js:498; test/plugins/openclaw-settings-attach.test.js
Why it matters: The pure prepareAttach no-op is proven, but nothing asserts a clean same-port reattach through attach() returns changed:false and leaves the file byte-identical (matters for mtime-gate churn).
Suggested fix: Add an attach() test capturing mtime before/after a second identical attach.

LLP 0109 is Status: Draft while its implementation is merging

Severity: nit
Confidence: 90
Evidence: llp/0109-openclaw-client-adapter.decision.md (header)
Why it matters: Repo LLP convention expects Accepted/Active on merge of a realized decision; no [inferred] claims remain (checked).
Suggested fix: Confirm author intends to flip status to Accepted/Active in this merge.

Inherited em dash on a reflowed doc-comment line

Severity: nit
Confidence: 85
Evidence: src/core/config/client_detach_disk.js:27
Why it matters: CLAUDE.md forbids U+2014 anywhere; this is reflowed pre-existing prose (the file is already saturated with em dashes on unchanged lines). All genuinely new files are em-dash-clean.
Suggested fix: Replace the em dash with a comma/parentheses on the touched line.

Reports: /Users/phil/workspace/hypaware/.git/worktrees/tmp.VLWZFJBqXn/dual-review/pr-317

…ession-id, dry-run refusal surfacing (LLP 0109) Addresses the final enumerated minor findings from round-3 review, all in adjacent files. The reviewed-clean reattach rework is untouched except for added test coverage. 1. Projector no longer clobbers a streamed empty-input tool_use call's input with '' (parseMaybeJson('')): finalizeBlock only overwrites block.input when the stream actually carried input_json_delta bytes, otherwise preserves the content_block_start input (default {}). 2. openclawSessionId no longer throws on a content-less first message with no system prompt (sha256Hex(undefined)): the fallback degrades to the exchange id via JSON.stringify(...) ?? exchangeId. 3. Dry-run now always runs the pure transform (with a harmless placeholder port when the endpoint is 127.0.0.1:0) so a refusal such as a non-Anthropic primary is surfaced in the plan instead of reporting success. 4. Added reattach-rework coverage (no code change): same-port malformed-marker refusal, untouched same-port no-op (no write, mtime unchanged), and api/apiKey provider-field drift heal. 5. Replaced the inherited em dash on the touched client_detach_disk.js doc line with a colon per CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

philcunliffe · 2026-07-15T23:44:41Z

neutral review — head 3c92d4d: clean, no actionable findings remain.

This closes the review loop for PR #317 (adopt, full-heal). Basis for this head's record:

Three full Codex+Claude dual-review rounds on the predecessors (dc98d5c, ab01bcb, 3fab02f). Round 3 gave the reworked reattach() path a targeted 7-risk deep dive and cleared it unanimously (no blocker/major/minor in the new code). The one design question, whether wholesale provider-entry replacement could clobber legitimate fields, was answered: models.providers.hypaware is 100% hypaware-owned (attach creates it, core detach removes it wholesale, OpenClaw caches resolved providers in a separate file), so wholesale replace is correct.
This head is the round-3 minor fixes, verified by a targeted read of the 3fab02f..3c92d4d delta:
- projector.js finalizeBlock now preserves a streamed empty-input tool_use call instead of clobbering input with ''.
- projector.js openclawSessionId guards JSON.stringify(content) ?? exchangeId so a content-less first message degrades to a stable key instead of throwing and dropping the exchange.
- settings.js attach --dry-run always runs the pure transform (throwaway placeholder port only for legality) so a refusal is surfaced even on a 127.0.0.1:0 endpoint; the plan still reports the real port.
- Added reattach-rework tests (malformed-marker refusal, true no-op, api/apiKey heal). Each new bug fix carries a fail-before/pass-after test.

CI green (typecheck + test) at this head; npm test shows only the 2 known pre-existing claude-backfill.test.js env failures (zero new). No CLAUDE.md violations introduced (this PR removes one pre-existing em dash and adds none).

Deferred to you (maintainer, not blocking): LLP 0109 is still Draft — consider promoting it (it ships with this PR). The remaining client_detach_disk.js em dashes are pre-existing on master (out of scope).

Advisory: the mechanical dual-review verdict reads request_changes purely from the medium blast-radius risk class (a new plugin touching several hot paths), which is inherent to the change and does not go away with findings resolved. No finding drives it. Weigh the blast radius at merge.

philcunliffe · 2026-07-15T23:44:58Z

neutral verdict: approved (adopt, full-heal) at head 3c92d4d.

Mergeable, CI green, reviewed with no actionable findings remaining. All findings from three review rounds were healed on-branch (the MAJOR silent-capture-on-config-drift class closed comprehensively via a shared buildProvider/reconcileProvider, plus the projector empty-input, session-id-fallback, and dry-run-validation minors), each with fail-before/pass-after tests.

Held for you to merge — neutral does not merge or ready a contributor's PR. Squash at merge; the LLP 0109 Draft-status promotion and the inherent medium blast-radius are yours to weigh (see the review record above).

philcunliffe · 2026-07-16T16:48:18Z

Review of head 4a4650a — clean. Verdict: approve.

The only change since the previously-approved head 3c92d4d is a single-line, author-owned LLP lifecycle promotion:

llp/0109-openclaw-client-adapter.decision.md
-**Status:** Draft
+**Status:** Accepted

The entire code surface (proxy routing, openclaw plugin config/index/projector/settings, bundled runtime, client-detach path, daemon status, json_util, and all tests) is byte-identical to 3c92d4d, which was reviewed clean and approved. git diff 3c92d4d..4a4650a touches only the one doc-status line.
Draft → Accepted is a legitimate author-owned lifecycle move (status is author-owned; Accepted = approved for implementation) by the doc's own author. No behavioral or code impact.
Head 4a4650a is green (typecheck + test SUCCESS) and MERGEABLE.

No actionable findings. Holding with neutral:approved for a maintainer to merge.

OpenClaw client adapter plugin + json_path attach probe format (LLP 0…

dc98d5c

…109)

platypii added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Jul 15, 2026

philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 15, 2026

Promote LLP 0109 to Accepted

4a4650a

Uh oh!

Conversation

platypii commented Jul 15, 2026 • edited by philcunliffe Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philcunliffe commented Jul 15, 2026

Dual-agent review — request_changes

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Fix Validations

cli_bundled_plugins_activated stale plugin counts

Findings

1) Behavioral Correctness

2) Contract & Interface Fidelity

9) Test Evidence Quality

No Finding

Evidence Bundle

Claude review

Inline import('...') type annotation in openclaw settings.js

Uh oh!

philcunliffe commented Jul 15, 2026

Uh oh!

philcunliffe commented Jul 15, 2026

Dual-agent review — request_changes

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

Fix Validations

Bundled smoke skipped-count drift

Findings

1) Behavioral Correctness

No Finding

Evidence Bundle

Claude review

Uh oh!

philcunliffe commented Jul 15, 2026

Dual-agent review — request_changes

Risk capstone

Cross-reference (reviewer findings vs surface)

Fix Validations

Bundled-plugin smoke skipped count drift

Re-attach heals marker-present capture drift

Findings

1) Behavioral Correctness

2) Contract & Interface Fidelity

No Finding

Evidence Bundle

Claude review

Projector overwrites tool-use input with empty string on streamed empty-input tool calls

Projector session-id fallback throws on a message with no content key

Dry-run can report success without reading/validating openclaw.json when endpoint is 127.0.0.1:0

Reattach-path MALFORMED_MARKER refusal is untested

attach()-level true-no-op (skip write, mtime unchanged) is untested

LLP 0109 is Status: Draft while its implementation is merging

Inherited em dash on a reflowed doc-comment line

Uh oh!

philcunliffe commented Jul 15, 2026

Uh oh!

philcunliffe commented Jul 15, 2026

Uh oh!

philcunliffe commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

platypii commented Jul 15, 2026 •

edited by philcunliffe

Loading

Dual-agent review — `request_changes`

Inline `import('...')` type annotation in openclaw settings.js

Dual-agent review — `request_changes`

Dual-agent review — `request_changes`

Projector session-id fallback throws on a message with no `content` key