Skip to content

Follow-up: deferred review findings from PR #815 #831

Description

@philcunliffe

Triage of PR #815 (integration/proxy-mode-capture, head b566a698) after its two review rounds found every residual finding non-blocking. Each item below was verified against the tree at that head; none can damage a tester's machine, produce a false pass of the claude_proxy_capture procedure, or contradict merged master. Deferred here rather than blocking the PR.

Deferred findings

1. openclaw_capture is missing from the AGENTS.md written-procedures list (pre-existing since #570). AGENTS.md:92-106 lists codex_desktop_capture and (post-#815) claude_proxy_capture, but the openclaw_capture procedure that has lived in docs/ACCEPTANCE.md since #570 is absent. One-line addition. Both review rounds noted it and deliberately left it as out of scope for a proxy-mode change set.

2. Two pre-existing broken anchors outside the diff. docs/ACCEPTANCE.md:228 cites LLP 0229#status-derives-by-the-same-gate but LLP 0229's anchors are context/decision/consequences/open-questions/references; llp/0231-proxy-mode-capture.rfc.md (body, two places) cites LLP 0114#interception-accepted but LLP 0114 carries no explicit anchors. Both predate #815; fixing the latter means editing an Accepted doc's body, which needs its own editorial decision.

3. claude_proxy_capture step 7's restore can leave a pre-rollout machine proxy-attached to a tunnel-only gateway. If the tester's backed-up config had proxy_mode off or absent (a pre-#794 install; post-rollout defaults are on, so most testers are unaffected), the documented finish (re-attach in proxy mode, then restore the backup and hyp daemon restart) ends with a proxy-mode attach against a gateway serving blind tunnels only. The machine keeps working by design (ai-gateway/src/source.js binds and serves tunnels exactly for this state, logs aigw.proxy_mode_stale_ca, and surfaces proxy_mode_error), Remote Control keeps working, but capture is silently off. The sentence "and hyp daemon restart after it if you want the machine back on its original mode" understates that the attach mode does not follow the config: while the CA remains on disk, a plain re-attach picks proxy mode again (claude/src/index.js, CA-presence preflight). The doc should say: to return fully to a proxy-off config, hyp detach claude --purge, restore the config, restart, then re-attach.

4. Step 1's CA check can race the async mint. docs/ACCEPTANCE.md step 1 runs ls ~/.hyp/hypaware/tls/ca-cert.pem immediately after hyp daemon restart; the gateway mints the CA after boot, so a fast operator can see a transient miss and misread it as "proxy mode is not being served". Step 7 already gates the same race with an explicit "wait for this before attaching" note; step 1 deserves the same one-line caveat. False-fail direction only, so the gate stays conservative.

5. Master's stale-CA warning names a remedy that cannot work (pre-existing master code, outside this diff). hypaware-core/plugins-workspace/ai-gateway/src/source.js (the aigw.proxy_mode_stale_ca warn) says "run hyp attach claude to move it back to base-URL mode", but attach selects the mode from CA presence, and in exactly this state the CA is still on disk, so re-attach stays in proxy mode. The working remedy is hyp detach claude --purge (or turning proxy_mode back on). Adjacent to open PR #822's territory; whoever lands that should sweep this message.

6. PR #791 overlap decision. #791 closes the LLP 0231 coverage gap from the code side while #815 closes it from the design side. Someone should decide whether #791 is still wanted once LLP 0245 lands as the coverage edge. Process item carried forward from both review rounds.

Verified clean at b566a698

All 18 findings from the two review rounds are confirmed fixed in the tree, including the four operator-safety ones (purge-then-attach downgrade per #819, step 7's attach/mint race, step 1's config backup/restore, and the LLP 0251 unmerged-number citations, now LLP 0242-0244). Every @ref/anchor in the diff resolves; the llp/0231 diff is exactly the one Designed-by: header line; LLP 0245 and 0248 are uniquely claimed across all remote branches; nothing asserted contradicts merged master.

Backlink: #815

Metadata

Metadata

Assignees

No one assigned

    Labels

    neutral:fixDelegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions