Skip to content

Follow-up: deferred review findings from PR #822 #832

Description

@philcunliffe

Triage of PR #822 (fix/issue-819, head 7bd43f63) after its two review rounds found no blocking residue. Every item below was re-verified against the tree at head, not taken from the review records. At head: npm test 4277 pass / 0 fail / 1 skipped, npm run typecheck clean, smokes client_attach_idempotent and status_diagnostics green. Both rounds' fixes are present and correct in the tree (src/core/commands/clients.js:374-380 carries the round-2 proxyStateChanged disqualifier; the remint/enabled/daemonInstalled reporting branches at clients.js:984-1010 and :1151-1180; the enabled: false guard at src/core/daemon/status.js:1237). These deferred items remain:

1. hyp status goes quiet again while a base-URL attach persists (medium, needs a new diagnostic)

proxy_mode_ca_missing (src/core/daemon/status.js:1235-1252) is keyed purely on config-vs-CA disagreement, so this sequence ends with every surface silent over a wrong-mode client:

  1. Stranded install (proxy_mode: true, no CA). hyp attach claude warns; the user declines the repair; a base-URL marker is written at the live port. All loud and correct so far.
  2. The daemon later restarts for any reason (reboot). prepareInterception re-mints the CA on launch (hypaware-core/plugins-workspace/ai-gateway/src/source.js:343-348), so proxy_mode_ca_missing clears and hyp status is healthy.
  3. hyp attach claude now no-ops at the LLP 0086 alreadyCurrent branch (src/core/commands/clients.js:374-380): the CA is present so the repair gate returns silently, proxyStateChanged is false, and the marker names the live port. The client stays on ANTHROPIC_BASE_URL, keychain trust is never re-granted, Remote Control inbound stays dead, exit 0.

Why deferred rather than a blocker on #822: the message printed in step 3 ("already attached") is incomplete, not false; the state is only reachable through an explicit, informed decline (on master today the same terminal state is reachable with zero warnings ever, so #822 strictly narrows it); and closing it needs data probeClientAttachFromDescriptor does not return - the probe carries version and port (src/core/daemon/status.js:~1676), not the attach mode the marker records. LLP 0259 #status-names-it deliberately settles its diagnostic as the config-versus-disk pair. The fix is a different diagnostic (attached-mode-vs-config, e.g. client_attach_mode_stale), plus optionally teaching the alreadyCurrent comparison to read the marker's recorded mode against the CA-implied one so re-attach repairs it. Round-2 record: "Worth a follow-up issue, not a change here."

2. Unconditional macOS / Remote Control wording in both proxy prompts (low, fix both or neither)

The repair prompt (src/core/commands/clients.js:~1120) says "macOS will then ask to trust the HypAware Local CA" and the downgrade warning (:1076-1080) says a base-URL attach "breaks Remote Control inbound", both unconditionally on every platform, while LLP 0237 makes Remote Control inbound darwin-only (the adapter itself says so elsewhere, hypaware-core/plugins-workspace/claude/src/index.js:241). The phrasing is copied verbatim from the pre-existing LLP 0244 migration prompt (clients.js:939-942), which has the same issue; diverging one of the pair would be worse than the shared inaccuracy. Fix all of them together, platform-gating the two claims.

3. maybeOfferProxyModeMigration's gateway lookup does not read enabled: false (low)

src/core/commands/clients.js:861-863 finds the gateway entry without the enabled !== false guard the status lookup gained in round 1 (status.js:1237). On a disabled gateway carrying stale proxy_mode: true with no CA, the repair is offered; accepting restarts the daemon for nothing and fails honestly at the bind wait ("could not restore proxy mode (... did not publish a bound port in time)"), falling back to base-URL attach. No misreporting, one wasted restart, in a state attach can barely reach. Add the same guard for symmetry.

4. hasLocalCa / attach all nits (info)

  • attach all prints the downgrade warning once per proxy-attach client; today exactly one manifest declares compose.gateway_proxy_mode (hypaware-core/plugins-workspace/claude/hypaware.plugin.json:45). Revisit when a second proxy-attach client lands.

5. PR #817 collision (medium, merge-time action for whichever merges second)

Recorded in detail from the other side in #830 (item 1); restated here so either merge order finds it:

#817 is unmerged, so today's tree needs nothing; this fires at the second merge.

6. Real-Mac acceptance items (both rounds; run before a release that ships this path)

Everything below the CA-file abstraction is Linux-provable; nothing above it is.

  1. The detach --purge then attach silently downgrades claude to base-URL mode, with no warning on any surface #819 repro end to end: attach in proxy mode, hyp detach claude --purge, hyp attach claude. Downgrade warning appears; accept; exactly one keychain trust dialog follows (Follow-up: F1 and F2 from #790, plus proxy-trust status findings deferred from PR #792 #793's two-dialog report is the neighbouring bug; this path must not add a third).
  2. Decline the repair, then run hyp attach claude a second time and accept. Confirm the client's settings.json actually flips from ANTHROPIC_BASE_URL back to HTTPS_PROXY + NODE_EXTRA_CA_CERTS, and that keychain trust is re-granted (the round-2 finding; on Linux only provable through the fake adapter).
  3. hyp status on the stranded machine before the repair: proxy_mode_ca_missing present, overall: healthy, proxy trust block absent.
  4. After the repair: launchd NODE_USE_SYSTEM_CA set, and Remote Control inbound actually alive.
  5. Both rounds recommend a docs/ACCEPTANCE.md entry for the sequence, since it is now a documented lifecycle state (LLP 0259) rather than an accident.

Backlink: PR #822. Review records: round 1 3b4776c6 and round 2 6294f0f4 comments on that PR. Cross-reference: #830 (item 1 there is the same #817 merge-time action).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    neutral:stuckneutral attempted this but cannot complete it autonomously — needs a human

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions