Skip to content

The stale-CA warning names a remedy that cannot work, and AGENTS.md omits openclaw_capture (#831) - #833

Merged
philcunliffe merged 3 commits into
masterfrom
fix/issue-831
Aug 19, 2026
Merged

The stale-CA warning names a remedy that cannot work, and AGENTS.md omits openclaw_capture (#831)#833
philcunliffe merged 3 commits into
masterfrom
fix/issue-831

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Works the two findings in #831 that are reachable from master today, and records why the rest are not. Gates: npm test 4257 pass / 0 fail / 1 skipped, npm run typecheck clean, both at this branch's head.

Closed here

Finding 5, master's stale-CA warning names a remedy that cannot work. hypaware-core/plugins-workspace/ai-gateway/src/source.js warns aigw.proxy_mode_stale_ca when proxy_mode is off but an interception CA is still on disk, and the reason read "run hyp attach claude to move it back to base-URL mode, or hyp detach claude". Verified on master: the Claude adapter reads readLocalCaInfo and sets mode: MODE_PROXY whenever a CA exists (hypaware-core/plugins-workspace/claude/src/index.js, the @ref LLP 0232#proxy-attach-preflight preflight), and LLP 0232 settles that explicitly ("attach uses proxy mode when a CA exists and base-URL mode otherwise"). In this exact state the CA is present, so a re-attach re-selects proxy mode and the operator is where they started. Plain hyp detach claude does not help either: purgeProxyTrustResidue is gated on --purge (src/core/commands/clients.js:149, LLP 0238 #ca-survives-detach), so only --purge calls deleteLocalCa.

The warning now names the two remedies that land: hyp detach claude --purge followed by a re-attach, or turning proxy_mode back on. The surrounding comment said the same wrong thing and was corrected with it, and carries a [constrained-by] ref to LLP 0232 #proxy-attach-preflight so the next reader sees why a re-attach cannot be the answer.

The existing blind-tunnel test in test/plugins/ai-gateway-proxy-mode.test.js asserted only that the event fired. It now captures the log attributes and pins the reason: hyp detach claude --purge must appear, and the old re-attach sentence must not. Perturbing the string back goes red.

Finding 1, openclaw_capture missing from the written-procedures list. docs/ACCEPTANCE.md:173 has carried ## openclaw_capture since #570, but AGENTS.md's "Written acceptance procedures" list named only codex_desktop_capture. Added, in the same shape as the existing bullet.

Merge note: #815 appends a claude_proxy_capture bullet at the same point in that list, so a trial merge conflicts in AGENTS.md. Resolution is keep both bullets in either order; nothing else in the file is touched by either side.

Not closed, and why

Finding 2 is factually wrong as filed; nothing to fix. Both cited anchors resolve on master. llp/0229-status-derives-attach-state-by-the-desired-gate.decision.md:54 defines <a id="status-derives-by-the-same-gate"></a>, and llp/0114-gateway-default-listen-port-fixed.decision.md:103 defines <a id="interception-accepted"></a> (LLP 0114's own Extended-by: line cites it by that name too). The finding reasoned from heading anchors alone. Inline <a id> anchors are the corpus's third anchor form and are explicitly indexed by ref-check (its Build the LLP index step measures 266 references resolving through that form), so neither citation is broken. No edit to llp/0231 was needed, which is just as well: it is Accepted.

Findings 3 and 4 are unreachable from master. Both are steps of the claude_proxy_capture procedure, which exists only in docs/ACCEPTANCE.md on origin/integration/proxy-mode-capture (PR #815, open). grep -r claude_proxy_capture over master returns nothing, so there is no step 1 and no step 7 here to edit. Work them after #815 merges.

Finding 6 is a process decision, not a code change. Whether #791 is still wanted once LLP 0245 lands is a call for a human, and LLP 0245 is also #815-only.

Fixes #831

…mits openclaw_capture (#831)

Two independently reachable findings from the #815 triage, both pre-existing
on master and outside that PR's diff.

`aigw.proxy_mode_stale_ca` fires when `proxy_mode` is off but an
interception CA is still on disk, and told the operator to "run `hyp attach
claude` to move it back to base-URL mode". Attach derives its mode from that
same CA (LLP 0232 #proxy-attach-preflight), so in exactly this state a
re-attach picks proxy mode again: the named remedy is a no-op. The warning
now names the two that land, removing the CA with `hyp detach claude
--purge` and re-attaching, or turning `proxy_mode` back on. The existing
blind-tunnel test pins the reason string so the wrong remedy cannot come
back.

AGENTS.md's "Written acceptance procedures" list has named only
`codex_desktop_capture` since #570 added `openclaw_capture` to
docs/ACCEPTANCE.md, so the second written procedure was invisible to anyone
reading the repo guidance rather than the doc. Listed it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Review round 1 - clean

Reviewed the three-file diff at f78e4445 in a detached worktree off origin/fix/issue-831. Nothing actionable found; nothing pushed. Gates re-run here, matching the PR body: npm test 4257 pass / 0 fail / 1 skipped, npm run typecheck clean, smokes client_attach_idempotent and status_diagnostics green.

The substantive claim holds: the old remedy really could not work

Verified from the code rather than from the PR body.

  • hypaware-core/plugins-workspace/claude/src/index.js:200-208 - the attach path reads readLocalCaInfo and spreads ...(ca ? { mode: MODE_PROXY, caCertPath: ca.certPath } : {}). Mode is a pure function of CA presence; config is never consulted. The dry-run branch at :189 does the same. There is no refusal path here: the only CA_MISSING in the adapter is hypaware-core/plugins-workspace/claude/src/settings.js:187, which sits downstream of proxy mode having already been selected.
  • src/core/commands/clients.js:149 - if (parsed.purge && !parsed.dryRun) is the sole caller of purgeProxyTrustResidue, which is the sole caller of deleteLocalCa (:1607). Plain hyp detach claude never removes the CA (@ref LLP 0238#ca-survives-detach).

So in the exact state the warning fires in (config proxy_mode off, CA on disk), hyp attach claude re-selects MODE_PROXY, the gateway still serves blind tunnels only, capture is still off, and the warning fires again on the next boot. The operator is returned precisely to where they started. The old text's fallback, plain hyp detach claude, is no better: it strips HTTPS_PROXY so capture stops entirely and the CA survives, so the warning never clears. Both halves of the old sentence were dead ends. The finding is real.

The replacement remedies both land

  • hyp detach claude --purge then re-attach. --purge deletes the CA, so the next readLocalCaInfo returns nothing and attach takes the base-URL branch - which is the correct end state, because config has proxy_mode off. The gateway's stale-CA branch (source.js:432) then stops firing, since it is gated on readLocalCaInfo returning something. hyp detach claude --purge is a valid invocation: parseClientArgs (src/core/commands/clients.js:1526-1584) accepts a positional client name alongside --purge.
  • I specifically checked the re-attach does not bounce the operator back into the stale state. maybeOfferProxyModeMigration does fire on that re-attach (config proxy_mode is not true), but it is an interactive offer, and its accept path runs enableGatewayProxyMode, which writes the config, restarts, and waits for the CA. Accepting therefore lands in a coherent proxy-mode install, not back in the degraded one. Declining lands in base-URL mode. Neither outcome is a loop.
  • Turning proxy_mode back on. The gateway then takes the interception branch, reuses the CA already on disk, and the client's existing HTTPS_PROXY attach becomes live again. Also lands.

Neither named remedy is the same defect in new clothes.

The reason-string pin is genuinely load-bearing

Verified by perturbation, not by reading. I reverted source.js:448-451 to master's exact string ('run \hyp attach claude` to move it back to base-URL mode, or `hyp detach claude`'`) and re-ran the file:

not ok 16 - proxy mode turned off with a CA still installed serves blind tunnels
  name: 'AssertionError'
# pass 17
# fail 1

Unperturbed the same file is 18/18. The worktree was restored to f78e4445 afterwards (git status clean). The pin fails when the old sentence returns, which is what a pin is for. The record helper change (test/plugins/ai-gateway-proxy-mode.test.js:559-563) is what makes it possible - the old helper discarded the attrs argument, so the reason was unobservable.

@ref honesty

@ref LLP 0232#proxy-attach-preflight resolves: llp/0232-claude-attaches-by-proxy.decision.md:47 is ### Proxy-attach preflight. Both new refs use it correctly.

  • [constrained-by] on the gateway branch (source.js:438) is the right relation and a genuinely non-obvious link: the gateway file contains nothing that would tell a reader why a re-attach cannot be the remedy, and 0232's preflight is exactly the doc that forecloses it.
  • [tests] on the assertion block is honest - the assertions do pin the consequence of that section.

Consistency with the corpus: LLP 0232 #proxy-attach-preflight self-contradicts, saying both "a missing CA is a refusal (markActionRefused), not a warning" and "attach uses proxy mode when a CA exists and base-URL mode otherwise". Open PR #822 mints LLP 0259 settling it in favour of the fallback. This PR's wording depends on the fallback reading (purge the CA, re-attach falls back to base URL) and is therefore consistent with the resolution rather than the retracted half - which also matches the code as built. Had it leaned on the refusal reading, the remedy would have been broken. No Accepted doc is edited here.

No overlap with #822's file set (clients.js, gateway_proxy_enable.js, status.js, plus three LLPs and three tests); the only shared file with #815 is AGENTS.md, which the PR body already records as the expected trivial conflict.

AGENTS.md bullet matches the real procedure

docs/ACCEPTANCE.md:173 is ## \openclaw_capture`, and every claim in the new bullet checks out against it: OpenClaw 2026.4.24 floor (:210), credentials for both anthropicandopenai (:203-207), both lanes with live gateway capture plus the periodic transcript sweep (:175-183), one row rather than two (:182-183), and reversibility via hyp detach (:183-184, step at :382). CLAUDE.mdis a symlink toAGENTS.md`, so the addition is visible from both names.

Findings deferred in the PR body, spot-checked

Finding 2 of #831 is indeed factually wrong: llp/0229-...:54 carries <a id="status-derives-by-the-same-gate"></a> and llp/0114-...:103 carries <a id="interception-accepted"></a>. The PR's refusal to edit an Accepted doc on a bad premise is right.

Style

No em dashes anywhere in the diff, no semicolons, JSDoc types only. The 145-char @ref gloss line is well inside this repo's norm for gloss lines (the longest in-tree is 387 chars).

Non-blocking nit, not fixed

The reason string's second remedy, "or turn proxy_mode back on", does not say that a hand-edited config needs a hyp daemon restart before it takes effect. In practice the operator who runs hyp attach claude gets the restart done for them by the LLP 0244 migration accept path, and the old message named no restart either, so this is not a regression and not worth churning a newly pinned string over. Recorded here rather than changed.

Tooling note

The code-review skill was launched against this PR but had not returned by the end of the round, so nothing from it is relied on here; every claim above is from first-hand reading, execution, and the perturbation run.

@philcunliffe philcunliffe added neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) and removed neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) labels Aug 18, 2026
Conflict was AGENTS.md's "Written acceptance procedures" list. #818 (LLP
0262, the OTEL telemetry attach change set) already added an
`openclaw_capture` bullet there, plus a new `claude_otel_shape_check`
bullet. This branch's Finding 1 was the same addition, so master's already
covers it. Took master's two bullets and dropped this branch's duplicate
rather than listing `openclaw_capture` twice.

`hypaware-core/plugins-workspace/ai-gateway/src/source.js` merged textually
but conflicted in meaning. The branch's Finding 5 fix argued the stale-CA
warning must not say "re-attach", because attach read its mode off the CA
and so re-selected proxy mode while the file was on disk. After #818 that
reason is gone: `hyp attach claude` is OTEL-only and explicitly never
re-attaches by proxy (LLP 0258 #version-floor, LLP 0262 #migration), and
LLP 0232's proxy-attach preflight is superseded for this client. The
conclusion still holds for a different reason, so the remedy text is kept
and the reasoning is restated on the ground that survives: attach leaves
the CA on disk on purpose, offering the trust back rather than taking it,
so a re-attach cannot clear this state. `hyp detach claude --purge` is the
remedy merged master's own migration notice prints. The `@ref` moves from
LLP 0232#proxy-attach-preflight to LLP 0262#migration in both the source
and the test that pins the message, since that is the decision now
constraining the behavior.
philcunliffe added a commit that referenced this pull request Aug 18, 2026
…#853)

`test/core/repo-scratch-hygiene.test.js` has been failing on `master` since it
merged: `x/npm-test.log` and `x/typecheck.log` are tracked, and the test's first
half asserts no `.log` is. Both came in on `adb448ab` (#785) via the `git add -A`
sweep that #786 wrote this test to catch; the files predate the test, so it was
red on arrival. Every branch cut since inherits it, currently blocking #833,
#849, #850 and #851 for a reason none of them caused.

The transcripts are deleted rather than the test relaxed, which is what its
message asks for. `.gitignore` needs nothing: `*.log` is already committed and
the rule test already passes, since an ignore rule cannot reach a path that is
already tracked. That asymmetry is the whole reason the file carries two tests.

Scope is exactly the two `.log` paths. The other eight files under `x/` are
untouched: the hygiene test forbids tracked transcripts and nothing else, and
sweeping up scratch it does not name would be a judgement this fix has no
authority to make.

Fixes #852

Co-authored-by: test <test@test.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review: approve, no actionable findings

Reviewed the PR's own delta against the merge base (b1afa7c2), which is origin/master: two files, +26/-7. The merge commit at the head (01df0852, absorbing #853's transcript deletion) is CI infrastructure and was excluded from review.

Gates re-run at 01df0852 in a clean worktree: npm test 4483 pass / 0 fail / 1 skipped; npm run typecheck clean. No em dashes, no semicolons, JSDoc-only types.

The core claim holds

I verified the defect and the fix independently rather than taking the PR body's word for it:

  • hyp attach claude never removes the CA. hypaware-core/plugins-workspace/claude/src/index.js:284-315 prints Run 'hyp detach claude --purge' to remove it and offers the trust back rather than taking it; deleteLocalCa is imported in exactly one place, src/core/commands/clients.js:18, reached only through purgeProxyTrustResidue behind the --purge gate at src/core/commands/clients.js:149. So the retired remedy really was a no-op in exactly the state that fires the warning.
  • Post-LLP-0262 the claude adapter always attaches in MODE_OTEL (index.js:206, :231, :361), so the retired text's "move it back to base-URL mode" was wrong on a second axis too.
  • Both named remedies actually land. hyp detach claude --purge runs the purge after the detach loop and unconditionally on the flag (clients.js:149-155), so it clears the CA even on a machine where claude was never attached. Turning proxy_mode back on takes the other branch of prepareInterception and the warning stops.
  • The @ref LLP 0262#migration [constrained-by] annotation resolves (llp/0262-otel-attach-replaces-proxy.rfc.md:180, ### Migration) and says what the gloss claims: attach "offers detach --purge for the CA trust".
  • No stale copy of the retired wording survives anywhere else in the tree.

The new string is also consistent with what the claude adapter already prints to the operator at index.js:315, so the two surfaces now agree.

Non-blocking observations, deliberately not changed

1. Info. hypaware-core/plugins-workspace/ai-gateway/src/source.js:433, the comment's opening premise is looser than the code. "a CA on disk means a client was attached in proxy mode at some point" is not quite right: ensureLocalCa has a single caller, source.js:485, so the gateway mints the CA whenever proxy_mode is on, independent of any attach. That premise line is pre-existing and unchanged by this PR, and the operational point it supports (a client may still have HTTPS_PROXY aimed here) is correct, so I left it. The related worry that hyp detach claude --purge silently reverses a claude OTEL attach as a side effect does not survive reading the full string: the message names the re-attach in the same breath, and the purge is machine-global but reachable only through a client-scoped detach, so there is no better command to name.

2. Low, test robustness. test/plugins/ai-gateway-proxy-mode.test.js:592. The negative guard assert.doesNotMatch(reason, /run \hyp attach claude` to move it back/)is pinned to the retired sentence verbatim, so it does not by itself enforce the property the adjacent comment states. I attempted to replace it with an ordering assertion (the purge must be named before any re-attach) and backed the change out: the current reason string legitimately says "re-attaching leaves this CA on disk" *before* naming the purge, as the explanation for why the purge is needed, so that assertion false-positives on correct text. More to the point, the positiveassert.match(reason, /hyp detach claude --purge/)` on the line above already fails any regression that drops the working remedy, which is the whole defect. The negative guard is belt-and-braces on top of a pin that already holds, so tightening it is a preference, not a fix, and I do not change tests on preference.

Verdict

Approve as-is. Nothing pushed to fix/issue-831; the branch is unmodified at 01df0852.

Two notes on the PR description, neither a code issue and neither worth an edit to the body: it says the annotation carries a [constrained-by] ref to LLP 0232 #proxy-attach-preflight, but the committed ref is LLP 0262#migration, and the committed one is the correct choice now that 0232's preflight is gone from the claude adapter. Its "Finding 1" AGENTS.md bullet for openclaw_capture is already present at the merge base (AGENTS.md:99), so its absence from the diff is right, not an omission.

@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 19, 2026
@philcunliffe
philcunliffe merged commit 1215492 into master Aug 19, 2026
9 checks passed
@philcunliffe
philcunliffe deleted the fix/issue-831 branch August 19, 2026 07:05
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.

Follow-up: deferred review findings from PR #815

1 participant