Skip to content

Proxy mode is the default and hyp attach claude migrates old installs (LLP 0242-0244) - #794

Merged
philcunliffe merged 3 commits into
masterfrom
feat/proxy-mode-default-and-migration
Aug 17, 2026
Merged

Proxy mode is the default and hyp attach claude migrates old installs (LLP 0242-0244)#794
philcunliffe merged 3 commits into
masterfrom
feat/proxy-mode-default-and-migration

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Fresh installs attached Claude by base URL because nothing wrote proxy_mode: true into the gateway config: the gateway needs the key to mint the CA, and attach preflights on the CA existing (LLP 0242). Proxy-mode capture itself shipped on the #782 line; this PR makes it the default and gives old installs a migration path.

What changed

  • Picker composes proxy_mode (LLP 0243). A proxy-capable client row declares gateway_proxy_mode in its plugin manifest (the claude row does), and composePickerConfig writes proxy_mode: true onto the gateway entry when such a row is picked. The hyp init claude preset writes the key literally.
  • hyp attach claude migrates old installs (LLP 0244). On a base-URL install it offers a consented y/N migration: enableGatewayProxyMode (new, src/core/config/gateway_proxy_enable.js) does a guarded local config write, daemon restart, bind wait, and CA wait. A central-managed gateway declines with central_managed - central ownership is the central layer naming the plugin, not local-entry absence, so a dead local write can never masquerade as a migration (found live on a fleet-joined machine). Non-TTY runs print one pointer note instead of prompting.
  • Finale CA race fixed.runPickerFinale waits for the CA (waitForLocalCa) before attaching on a proxy-mode config, so fresh installs can no longer silently fall back to base-URL because the daemon had not minted yet.
  • LLP 0233 gains Extended-by: LLP 0244.

Testing

  • npm test under node 22.22.0 with an isolated HYP_HOME: 4185/4202 pass; the 15 failures are the pre-existing parquet/iceberg NULL cluster, verified identical on a clean tree.
  • New: gateway-proxy-enable, attach-proxy-migration, init-proxy-mode-default; updated: compose-picker-config, attach-enable-*, attach-enablement-state, plus the claude_attach_detach and walkthrough_picker_to_first_query smokes.
  • npm run typecheck and npm run build:types green.
  • Live-accepted today on a fleet-managed machine: fleet config with proxy_mode: true pulled, CA minted, keychain grant via hyp attach claude, CONNECT interception verified against api.anthropic.com, capture rows continuous through the swap.

Note (not this PR): the ai-gateway source tests read the real ~/.hyp CA through defaultStateRoot when the test ctx pins no HYP_HOME, so 6 of them fail on any machine with a minted CA (green in CI). Worth a follow-up isolation fix.

🤖 Generated with Claude Code

… (LLP 0242-0244)
Fresh installs attached Claude by base URL because nothing wrote
proxy_mode: true (LLP 0242): the gateway needs the key to mint the CA,
and attach preflights on the CA existing.
- The picker composes proxy_mode onto the gateway entry whenever a
proxy-capable client row asks for it; the claude row declares
gateway_proxy_mode in hypaware.plugin.json and the hyp init claude
preset writes the key literally (LLP 0243).
- hyp attach claude is the migration verb for existing base-URL
installs: a consented y/N prompt drives enableGatewayProxyMode
(guarded local config write + daemon restart + bind wait + CA wait).
A central-managed gateway declines with central_managed - central
ownership means the central layer naming the plugin, not local-entry
absence, so a dead local write can never look like a migration.
Non-TTY runs print one pointer note instead of prompting (LLP 0244).
- runPickerFinale waits for the CA before attaching on a proxy-mode
config, closing the race where fresh installs silently attached
base-URL because the daemon had not minted yet.
- LLP 0233 gains the Extended-by: LLP 0244 forward ref.
Suite green under an isolated HYP_HOME except the pre-existing
parquet/iceberg NULL cluster (15, verified on a clean tree).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

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

SourceFinding (severity, evidence)Intersects
CodexReconfigure carry-forward acquires proxy_mode without consent (major, walkthrough.js:1006/1262)Config field chain; Risks 1
CodexCentral-managed check resolves layers without catalog metadata (major, clients.js:826, gateway_proxy_enable.js:120)Config field chain (central outranks local); Risks 3
Codexoutcome stays 'enabled' on post-write failure (minor, gateway_proxy_enable.js:190)Concurrency surface; Risks 4
ClaudeFinale CA-wait branch executed by no test or smoke (major, walkthrough.js:1611-1620)Direct callers (runPickerFinale); Risks 2
ClaudeFinale CA-wait stalls 15s on fleet-managed gateways (minor, walkthrough.js:1611-1619)Risks 3
ClaudeLLP 0244 #enable-write promises entry creation code refuses (minor, llp/0244:50)Targets (LLP 0244)
Claudehyp attach all silent on migratable install (minor, clients.js:824)Direct callers (maybeOfferProxyModeMigration)
ClaudewaitForLocalCa never runs under test (minor, ca.js:394)Direct callers (waitForLocalCa)
ClaudeMigration failure paths asserted nowhere (minor, clients.js:308-313)Risks 4
Claudeproxy_mode: false opt-out survives only by generic merge, untested (minor, walkthrough.js:1260-1264)Config field chain
Codex review

Fix Validations

Fresh Claude installs compose proxy_mode: true

  • Status: correct
  • Evidence: hypaware-core/plugins-workspace/claude/hypaware.plugin.json:45, src/core/cli/walkthrough.js:973, src/core/cli/walkthrough.js:1006, hypaware-core/plugins-workspace/claude/src/index.js:413
  • Assessment: The manifest declares the new row capability, the picker fold writes the gateway key, and the literal Claude preset was updated too. Fresh compose coverage exists in test/core/init-proxy-mode-default.test.js:44 and test/core/compose-picker-config.test.js:65.

hyp attach claude offers a consented migration

  • Status: incomplete
  • Evidence: src/core/commands/clients.js:306, src/core/commands/clients.js:821, src/core/config/gateway_proxy_enable.js:166, src/core/config/gateway_proxy_enable.js:221, src/core/config/gateway_proxy_enable.js:251
  • Assessment: The single-client interactive path prompts, writes, restarts, waits for bind, and waits for the CA. See findings below for two contract gaps around old-config reconfigure and central-layer detection.

Picker finale waits for the local CA before attach

  • Status: correct
  • Evidence: src/core/cli/walkthrough.js:1601, src/core/cli/walkthrough.js:1612, src/core/cli/walkthrough.js:1613, hypaware-core/smoke/flows/walkthrough_picker_to_first_query.js:626
  • Assessment: Proxy-mode walkthrough installs now wait for the CA before attaching clients, with a bounded warning fallback if the daemon does not mint in time.

Findings

2) Contract & Interface Fidelity

  • Severity: major
  • Confidence: high
  • Evidence: llp/0243-picker-composes-proxy-mode.decision.md:61, llp/0243-picker-composes-proxy-mode.decision.md:62, src/core/cli/walkthrough.js:1006, src/core/cli/walkthrough.js:1046, src/core/cli/walkthrough.js:1262, src/core/cli/wizard/pick.js:387
  • Why it matters: An existing Claude install that re-runs the picker can acquire proxy_mode: true through carry-forward composition, bypassing LLP 0244’s explicit y/N migration prompt.
  • Suggested fix: Special-case gateway proxy_mode during carryForwardExistingConfig: on args.existing, preserve an absent prior key as absent, just as false is preserved, unless the reconfigure flow adds its own explicit migration consent and tests.

2) Contract & Interface Fidelity

  • Severity: major
  • Confidence: medium
  • Evidence: src/core/commands/clients.js:826, src/core/commands/clients.js:858, src/core/commands/clients.js:859, src/core/config/gateway_proxy_enable.js:120, src/core/config/gateway_proxy_enable.js:123, src/core/runtime/boot.js:381
  • Why it matters: The pre-prompt central-managed check resolves layers without the catalog metadata it already built, so fleet configs needing installed plugin/dataset metadata can fail resolution, suppress the central guard, and show a migration prompt whose “yes” cannot land locally.
  • Suggested fix: Pass catalog.pluginMetadata and catalog.knownDatasets into resolveLayeredConfigFromDisk in maybeOfferProxyModeMigration, matching enableGatewayProxyMode, and add a central-config test with a non-default known plugin or dataset.

11) Debuggability & Operability

  • Severity: minor
  • Confidence: high
  • Evidence: src/core/config/types.d.ts:827, src/core/config/types.d.ts:829, src/core/config/gateway_proxy_enable.js:190, src/core/config/gateway_proxy_enable.js:291, src/core/config/gateway_proxy_enable.js:293, src/core/commands/clients.js:922
  • Why it matters: Bind/CA/restart failures after the write leave outcome: 'enabled' even though the public result contract has a failed outcome, making logs and downstream checks ambiguous.
  • Suggested fix: Set result.outcome = 'failed' in fail(), and add a separate writePersisted flag or rely on steps.write === 'ok' for partial-write reporting.

No Finding

  1. Behavioral Correctness
  2. Change Impact / Blast Radius
  3. Concurrency, Ordering & State Safety
  4. Error Handling & Resilience
  5. Security Surface
  6. Resource Lifecycle & Cleanup
  7. Release Safety
  8. Test Evidence Quality
  9. Architectural Consistency

Evidence Bundle

  • Changed hot paths:composePickerConfig, runPickerFinale, runClientLifecycle, maybeOfferProxyModeMigration, enableGatewayProxyMode, waitForLocalCa, Claude picker manifest, Claude init preset.
  • Impacted callers: src/core/cli/wizard/pick.js:387, src/core/cli/walkthrough.js:727, src/core/commands/clients.js:306, src/core/commands/clients.js:911.
  • Impacted tests: test/core/compose-picker-config.test.js:65, test/core/init-proxy-mode-default.test.js:44, test/core/attach-proxy-migration.test.js:25, test/core/gateway-proxy-enable.test.js:252, test/core/attach-enablement-state.test.js:279.
  • Unresolved uncertainty: I did not run tests. The central-layer finding depends on resolver validation needing catalog metadata for some fleet configs; the code already signals that by accepting knownPlugins/knownDatasets, but I did not build a reproducer.
Claude review

Claude review

Five parallel lenses: guidance compliance, shallow bug scan, historical context, contracts & callers, comments & tests. Guidance and shallow-bug lenses found no issues (em-dash/semicolon/JSDoc/@ref/LLP conventions verified, ref-hygiene test 11/11 on the PR head; no logic bugs visible in the diff).

The finale CA-wait branch is executed by no test or smoke

  • Severity: major
  • Confidence: 88
  • Evidence: src/core/cli/walkthrough.js:1611-1620
  • Why it matters: This is the PR's finale CA-race fix, yet every existing runPickerFinale test passes skipDaemon: true (walkthrough-attach-lane.test.js:66, client-assets-prune.test.js:1166, cli/wizard/progress.test.js:262) and the walkthrough smoke runs --dry-run and --no-daemon, so the guard, the wait, and the timeout warning are all dead in CI; the call site passes no timeoutMs/sleep, so the branch cannot be unit-tested without a real 15-second poll.
  • Suggested fix: Add a runPickerFinale test with skipDaemon unset, a proxy-mode config, and a stubbed daemon install; thread waitForLocalCa (or its timeoutMs/sleep) through runPickerFinale's args the same way enableGatewayProxyMode accepts waitForCaFn, and assert both the wait-before-attach ordering and the warning on timeout.

Wizard finale CA-wait stalls 15s on fleet-managed gateways

  • Severity: minor
  • Confidence: 82
  • Evidence: src/core/cli/walkthrough.js:1611-1619
  • Why it matters: The finale decides to wait from the local composed config alone (gatewayEntry?.config?.proxy_mode === true), but on a wizard run that joins a fleet whose central layer names @hypaware/ai-gateway, the LLP 0031 merge drops the local gateway entry, the daemon boots without proxy mode, no CA is minted, and every such enrollment burns the full 15s timeout and prints a warning recommending a repair (hyp attach) that only reports fleet-managed.
  • Suggested fix: Apply the same central-ownership check the other two call sites use before waiting (or wait on the effective gateway block's proxy_mode), emitting the fleet-managed note instead of the re-run-attach warning.

LLP 0244 #enable-write promises entry creation the code refuses

  • Severity: minor
  • Confidence: 85
  • Evidence: llp/0244-attach-migrates-to-proxy-mode.decision.md:50
  • Why it matters: The Accepted decision says the write sets the key on the existing local entry "(creating the entry only when no layer provides one)", but enableGatewayProxyMode returns no_gateway and writes nothing in that case, while the function and its tests carry @ref LLP 0244#enable-write - a settled doc misdescribes the shipped write shape and the refs are dishonest per the repo's own LLP rules.
  • Suggested fix: LLP 0244 is minted in this same PR (still editable pre-merge): drop the "(creating the entry...)" parenthetical so the doc matches the refuse-with-no_gateway behavior the code and tests pin; alternatively implement the creation branch.

hyp attach all contradicts LLP 0244 #non-interactive's "emits one warning" contract

  • Severity: minor
  • Confidence: 80
  • Evidence: src/core/commands/clients.js:824
  • Why it matters: The early client === 'all' return fires before the pointer-note branch, so hyp attach all on a migratable base-URL install emits nothing at all, diverging from the Accepted decision merged in this same PR; old installs whose habit is hyp attach all never learn the migration exists.
  • Suggested fix: Emit the same one-line non-prompt pointer in the all path (it is prompt-free, so it cannot violate the never-prompts posture), or add an explicit all/dry-run carve-out sentence to LLP 0244 #non-interactive before the doc ossifies; update the "hyp attach all never asks" test to pin whichever is chosen.

waitForLocalCa itself never runs under test

  • Severity: minor
  • Confidence: 82
  • Evidence: src/core/tls/ca.js:394
  • Why it matters: Both production call sites are in otherwise-untested paths and every unit test injects a fake, so the new exported polling loop's deadline math, throw swallowing, and return shape have zero coverage despite carrying injectable sleep/now hooks built for exactly this.
  • Suggested fix: Add a deterministic test in test/core that drives waitForLocalCa with fake sleep/now against a temp stateRoot: CA appears after N polls returns { ready: true, certPath }; never appears returns { ready: false } at the deadline.

Migration failure paths are asserted nowhere

  • Severity: minor
  • Confidence: 85
  • Evidence: src/core/commands/clients.js:308-313
  • Why it matters: LLP 0244's core safety promise, "a migration failure never fails the attach", has no test: nothing makes maybeOfferProxyModeMigration throw (the catch downgrading to a warning), nothing drives an accepted migration into result.ok === false at the attach level, and inside enableGatewayProxyMode the write-refused/write-throw and restartDaemon-throw branches are uncovered.
  • Suggested fix: Add one attach-level test where an injected failure proves attach still exits 0 with the warning, and two enableGatewayProxyMode unit tests for restartDaemon throwing and the guarded write failing.

LLP 0243 #user-key-wins ("proxy_mode: false survives reconfigure") has no test

  • Severity: minor
  • Confidence: 85
  • Evidence: src/core/cli/walkthrough.js:1260-1264
  • Why it matters: The Accepted decision and the new code comment both promise a hand-written proxy_mode: false outranks the composed default on a reconfigure; this holds today only because generic mergePlugin spreads prior.config last, and mergePlugin already special-cases gateway upstreams as composer-owned, so a future change would silently break the decided opt-out with nothing red.
  • Suggested fix: In test/core/compose-picker-config.test.js, compose ['claude'] with an existing config whose gateway entry carries proxy_mode: false and assert the merged gateway config keeps proxy_mode: false while upstreams is still recomposed.

Reports: .git/dual-review/pr-794

philcunliffeand others added 2 commits August 17, 2026 12:16
Review fixes on top of the LLP 0242-0244 change set (PR #794 dual-review,
verdict request_changes):
- Carry-forward preserves proxy_mode absence: an existing gateway entry
gains the key only through the LLP 0244 migration, so a picker re-run
no longer silently upgrades an old install (mergePlugin deletes the
composed key when the prior entry lacks it; LLP 0243 #user-key-wins
now states absence carries forward like a value).
- The finale CA wait is extracted into waitForProxyCaBeforeAttach, is
central-aware (a fleet-owned gateway block decides the mode, so a
dead local key no longer burns the 15s timeout), and is injectable
(runPickerFinale accepts waitForCaFn); covered by a new test file.
- The migration offer resolves config layers with the same catalog
metadata the accept path uses, so the two resolutions cannot disagree.
- hyp attach all emits the one-line migration pointer instead of
returning silently, matching LLP 0244 #non-interactive; the doc gains
the explicit all/dry-run sentences.
- enableGatewayProxyMode reports outcome 'failed' when a post-write
step breaks; steps.write plus backupPath carry the persisted-write
signal. LLP 0244 #enable-write now matches the refuse-with-no_gateway
behavior instead of promising entry creation.
- New coverage: waitForLocalCa deadline math (fake clock), restart-throw
and filesystem-refused-write failure paths, the attach-level
migration-failure-never-fails-attach promise, and the proxy_mode
false/absent carry-forward pins.
Suite: 4199/4216 pass under an isolated HYP_HOME (the 15 remaining are
the pre-existing parquet/iceberg NULL cluster); typecheck, build:types,
and the claude_attach_detach + walkthrough_picker_to_first_query smokes
green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- runPickerFinale gains an injectable installDaemonFn (the real one
refuses service managers under the test runner, LLP 0181), and a new
test drives the open-gate path end to end pinning the order: install,
CA wait, first attach. Deleting the wait call now fails a test.
- LLP 0243 #user-key-wins scopes "reconfigure" to the interactive lane:
a non-interactive --force re-init composes from scratch by design (its
output is byte-identical for the same inputs and the overwrite sits
behind an explicit --force), so it re-applies the composed default.
The walkthrough comment says the same where the key is written.
Suite 4200/4217 under an isolated HYP_HOME (15 remaining are the
pre-existing parquet/iceberg NULL cluster); typecheck, build:types, and
the walkthrough smoke green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Dual-agent review, round 2 — approve

  • Verdict:approve (round 1 was request_changes; its 3 majors and 7 minors were fixed in 525549f, verified by a closure audit this round)
  • Risk class:low
  • Head reviewed:525549f; the two residual minors this round found are fixed in 320265a (now at head)
  • Auto-merge advisory: 👎 thumbs down — approve verdict, but findings were raised and fixed in-flight (320265a) and one Codex finding required adjudication; human merge gate stands

Advisory only: no merge was attempted.

Adjudication note: Codex raised one cat-1 major (finale could attach base-URL after a CA-wait timeout) conditioned on its own unresolved uncertainty ("if installDaemon() itself starts the daemon ... the finding downgrades"). Verified false by direct inspection: installLaunchAgent runs launchctl bootstrap on a RunAtLoad plist and installSystemdUnit runs enable+restart, so the daemon starts (and begins minting the CA) at install time, before the wait; the 2026-08-17 fresh-wizard live run exercised exactly this path. Full trail in the codex.md adjudication section.

Risk capstone

Cross-reference: reviewer findings vs high-risk surfaces

SourceFinding (severity, evidence)Intersects
CodexFinale can attach base-URL after CA-wait timeout (major, walkthrough.js:1691) - ADJUDICATED FALSE: installDaemon starts the daemon on both platforms (launchctl bootstrap + RunAtLoad; systemctl enable+restart) before the wait, per Codex's own stated downgrade condition; fresh-wizard live run 2026-08-17 confirmsConfig field chain
ClaudeOpen-gate finale wiring untested (minor, walkthrough.js:1691-1698) - FIXED in 320265aDirect callers (runPickerFinale)
Claude#user-key-wins over-promised for non-interactive --force (minor, pick.js:118) - FIXED in 320265a (doc scoped)Config field chain
Codex review

Fix Validations

Fresh picker installs did not write proxy_mode

  • Status: correct
  • Evidence:hypaware-core/plugins-workspace/claude/hypaware.plugin.json:42, src/core/cli/walkthrough.js:968, src/core/cli/walkthrough.js:1008
  • Assessment: The Claude picker row now declares gateway_proxy_mode, and composePickerConfig folds that into proxy_mode: true on fresh gateway entries.

Existing hyp attach claude had no migration path

  • Status: correct
  • Evidence:src/core/commands/clients.js:821, src/core/commands/clients.js:903, src/core/config/gateway_proxy_enable.js:166, src/core/config/gateway_proxy_enable.js:221, src/core/config/gateway_proxy_enable.js:251
  • Assessment: Interactive attach now prompts, writes the local gateway key, restarts, waits for bind, and waits for CA. Central-managed decline is also pinned at src/core/commands/clients.js:875 and src/core/config/gateway_proxy_enable.js:153.

Finale CA race before fresh attach

  • Status: incomplete
  • Evidence:src/core/cli/walkthrough.js:1691, src/core/cli/walkthrough.js:1728, src/core/cli/walkthrough.js:1850, hypaware-core/plugins-workspace/claude/src/index.js:201
  • Assessment: The finale waits for the CA before attaching, but it still does so before the explicit daemon restart/start later in the finale. Any path where the CA is minted by that restart still attaches before the CA exists.

Findings

1) Behavioral Correctness

  • Severity: major
  • Confidence: medium
  • Evidence:src/core/cli/walkthrough.js:1691, src/core/cli/walkthrough.js:1728, src/core/cli/walkthrough.js:1850, hypaware-core/plugins-workspace/claude/src/index.js:201, hypaware-core/plugins-workspace/claude/src/index.js:208, hypaware-core/plugins-workspace/claude/src/index.js:273
  • Why it matters: Fresh setup can still write base-URL Claude settings after timing out the new CA wait, because the adapter only chooses proxy mode when the CA file already exists.
  • Suggested fix: Move the daemon start/restart that can mint the CA before waitForProxyCaBeforeAttach, then attach after a successful CA wait, or split proxy-mode finale ordering so Claude attach is deferred until after the daemon has actually started on the new config.

No Finding

  1. Contract & Interface Fidelity
  2. Change Impact / Blast Radius
  3. Concurrency, Ordering & State Safety
  4. Error Handling & Resilience
  5. Security Surface
  6. Resource Lifecycle & Cleanup
  7. Release Safety
  8. Test Evidence Quality
  9. Architectural Consistency
  10. Debuggability & Operability

Evidence Bundle

  • Changed hot paths:src/core/cli/walkthrough.js:953, src/core/cli/walkthrough.js:1510, src/core/commands/clients.js:821, src/core/config/gateway_proxy_enable.js:69, src/core/tls/ca.js:394
  • Impacted callers:src/core/cli/walkthrough.js:727, src/core/cli/walkthrough.js:790, src/core/cli/wizard/pick.js:387, src/core/commands/clients.js:73
  • Impacted tests:test/core/compose-picker-config.test.js:515, test/core/gateway-proxy-enable.test.js:70, test/core/attach-proxy-migration.test.js:143, test/core/init-proxy-mode-default.test.js:46, test/core/tls-ca.test.js:198, test/core/walkthrough-finale-ca-wait.test.js:93
  • Unresolved uncertainty: I did not open src/core/daemon/install.js due the 5-file review cap; if installDaemon() itself starts the daemon and mints the CA before src/core/cli/walkthrough.js:1691, the finale finding downgrades. The in-file finale ordering and comments still point to the explicit restart at src/core/cli/walkthrough.js:1850 as the relevant start point.

Orchestrator adjudication

The single finding above is refuted on the exact condition its own
Unresolved-uncertainty note named ("if installDaemon() itself starts the
daemon and mints the CA before walkthrough.js:1691, the finale finding
downgrades"), verified by direct inspection outside Codex's 5-file cap:

  • macOS: installLaunchAgent runs launchctl bootstrap on a plist with
    RunAtLoad (src/core/daemon/macos.js:304, macos.js:121), which loads AND
    launches the agent at install time.
  • Linux: installSystemdUnit runs systemctl enable + restart
    (src/core/daemon/linux.js:241-242), starting the unit at install time.

So the finale order is: install (daemon starts on the new proxy-mode config,
begins minting the CA) -> waitForProxyCaBeforeAttach -> attach. The
explicit restart at walkthrough.js:1850 is a config-take-effect refresh, not
the first start. Empirically, the fresh-wizard acceptance run of 2026-08-17
attached Claude in proxy mode through exactly this path. The finding is
excluded from the verdict as a verified false positive; the mechanical count
below it would otherwise have been codex_major=1.

Claude review

Claude review (round 2, against 525549f)

Four lenses: guidance compliance, fix-commit bug scan, round-1 closure audit, contracts & doc honesty. Guidance and bug-scan lenses found no issues (all seven new @ref anchors resolve; the mergePlugin delete, layering decision, attach-all fallthrough, and outcome change verified sound). The closure audit confirmed all nine round-1 findings genuinely fixed. Two residual minors survived, and both were fixed in the follow-up commit 320265a before this report was finalized.

Finale's open-gate CA-wait wiring ran under no test

  • Severity: minor
  • Confidence: 85
  • Evidence: src/core/cli/walkthrough.js:1691-1698
  • Why it matters: The extracted helper was thoroughly tested but no test executed runPickerFinale with skipInstall unset (installDaemon refuses service managers under the test runner and had no injection seam), so deleting the six-line wait call before the attach loop would have left every test and smoke green.
  • Suggested fix: An injectable daemon-install seam so one test runs the open-gate path and asserts the wait fires between install and the first attach.
  • Outcome: FIXED in 320265a - runPickerFinale gained installDaemonFn and a test pins the install -> CA wait -> attach order.

#user-key-wins held only for interactive reconfigures; hyp init --yes --force still re-applied the default

  • Severity: minor
  • Confidence: 82
  • Evidence: src/core/cli/wizard/pick.js:118, llp/0243-picker-composes-proxy-mode.decision.md
  • Why it matters: The carry-forward protection rides args.existing, which only the interactive lane supplies; LLP 0243 #user-key-wins claimed the protection unqualified, over-promising the consent property. (A non-interactive overwrite sits behind an explicit --force, which is why this is a doc-honesty minor, not a consent major.)
  • Suggested fix: Scope the doc (and the code comment) to the interactive lane, stating the --force whole-file overwrite is its own consent, or extend carry-forward to the non-interactive lane.
  • Outcome: FIXED in 320265a - LLP 0243 #user-key-wins and the walkthrough comment now scope "reconfigure" to the interactive lane and name the --force overwrite as the consent; the non-interactive from-scratch compose is the documented design (pick.js byte-identical rule).

Reports: .git/dual-review/pr-794

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Decision map

~2,500 diff lines, of which roughly two thirds are scaffolding you can skim: nine test files (the largest share), four LLP documents restating the decisions in prose, JSDoc/type threading for two new optional params, and the mechanical halves of the plumbing. The decision content concentrates in six forks. Ranked by stakes x skim-risk:

1. Proxy mode becomes the default for every fresh Claude install [unhappy-path policy / default-on]

src/core/cli/walkthrough.js:1014 - ...(gatewayProxyMode ? { proxy_mode: true } : {}) - driven by one manifest bit, claude/hypaware.plugin.json:45.
Alternative not taken: keep base-URL the default and make proxy an opt-in wizard question (or an opt-in flag).
Verify: you agree the CA-trust dialog remaining its own refusable consent moment (LLP 0237) is enough consent for the config default to flip; and that rows not declaring the bit (Codex) mint no CA they will never use.

2. "Centrally managed" means the central layer NAMES the gateway, not that the local entry is missing [contract]

src/core/config/gateway_proxy_enable.js:153 and the finale's mirror src/core/cli/walkthrough.js:1533 (centralGateway ?? local).
Alternative not taken: decide ownership from local-entry absence - which is what the first live migration attempt effectively did, producing a dead write (documented in the PR).
Verify: the ownership rule reads the same in all three places that apply it (enable, offer, finale wait), and central-with-proxy_mode still waits while central-without skips.

3. A picker re-run never upgrades an existing install; attach is the only migration verb [unhappy-path policy / consent]

src/core/cli/walkthrough.js:1278 - one delete config.proxy_mode preserving the key's absence through carry-forward.
Alternative not taken: treat re-running the picker as consent and let composition write the key onto existing gateway entries (round 1 shipped this; it was reversed).
Verify: the scope is stated honestly - interactive reconfigures only; a non-interactive --force re-init recomposes from scratch by documented design (LLP 0243 #user-key-wins, last paragraph).

4. CA wait: 15 seconds, and a timeout warns-and-continues instead of failing [magic value + unhappy-path policy]

src/core/config/gateway_proxy_enable.js:22 (CA_WAIT_DEFAULT_MS = 15_000), degradation text at src/core/cli/walkthrough.js:1544.
Alternative not taken: fail the attach/finale on timeout, or wait unbounded.
Verify: 15s comfortably covers daemon boot + CA mint on a slow machine, and you accept the failure mode (silent-ish base-URL attach plus one warning) over a hard stop - base-URL still captures, and re-running attach repairs.

5. The migration asks once, defaults No, and every non-migrating shape gets exactly one pointer line [contract]

Question at src/core/commands/clients.js:908 ([y/N]), the one-pointer branch at src/core/commands/clients.js:886 (all / --json / non-TTY).
Alternative not taken: auto-migrate on attach (no question), or keep attach all silent (round 1's behavior).
Verify: the exact stderr line is pinned by tests and one smoke golden-compares it, so its wording is now a compatibility surface you are happy to own.

6. A post-write failure reports outcome: 'failed'; the persisted write rides steps.write + backupPath[contract]

src/core/config/gateway_proxy_enable.js:295
Alternative not taken: keep outcome: 'enabled' once the write lands (round 1's shape), or add a dedicated writePersisted flag.
Verify: every consumer branches on ok && outcome === 'enabled', so nothing reads the old shape.

Honorable mentions: the hyp init claude preset writes the key literally rather than riding the fold; no_gateway refuses rather than inventing a gateway entry (LLP 0244 #enable-write); runPickerFinale grew installDaemonFn/waitForCaFn seams solely so the open-gate ordering is testable.


Generated from the dual-review artifacts at head 320265a.

@philcunliffe
philcunliffe merged commit 04330ab into masterAug 17, 2026
9 checks passed
@philcunliffe
philcunliffe deleted the feat/proxy-mode-default-and-migration branch August 17, 2026 19:35
philcunliffe pushed a commit that referenced this pull request Aug 17, 2026
…ign 0246
The design (LLP 0246) is already realized on master by 04330ab (#794);
the plan records the symbol-by-symbol conformance pass, enumerates the
nine consent/refusal pins that exist, and schedules the two outstanding
items: the missing --json non-interactive test pin (T1) and the corpus
cross-links binding 0242/0243/0244 to the design of record (T2).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
philcunliffe pushed a commit that referenced this pull request Aug 18, 2026
… fields hyp status drops
- The rollout is LLP 0242-0244 (04330ab, #794), on master; LLP 0251 is an
unmerged number on a sibling branch, so three references dangled.
- Section 5 listed proxy_mode, expiry, cert path and permitted/intercepted
hosts as hyp status output; ProxyTrustReport carries three fields and
hyp status --json drops the source details block those live in.
- Section 3's / anchor rationale was inverted: the fail-closed guard is what
stops record-everything, and the source.js merge is what stops the default
install recording nothing.
- 0231's forward-ref uses the corpus's Designed-by name.
- Acceptance: purge deletes the CA while the running daemon keeps its own, so
a purge-then-attach silently downgrades to base URL; the step-7 restore
raced the CA mint the same way. Config backed up and restored, anchors
folded into their links (#proxy-attach-preflight is LLP 0232's, not 0245's).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
philcunliffe added a commit that referenced this pull request Aug 19, 2026
…g --json non-interactive pin (#817)
* LLP 0246: proxy-mode default and attach migration design covers issue 0242
Design of record for LLP 0242's fix (LLP 0243/0244): the picker fold and
hyp init claude preset compose proxy_mode: true by default, and hyp attach
claude migrates existing base-URL installs behind an explicit consent
prompt via enableGatewayProxyMode.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* LLP 0249: plan for proxy-mode default and attach migration covers design 0246
The design (LLP 0246) is already realized on master by 04330ab (#794);
the plan records the symbol-by-symbol conformance pass, enumerates the
nine consent/refusal pins that exist, and schedules the two outstanding
items: the missing --json non-interactive test pin (T1) and the corpus
cross-links binding 0242/0243/0244 to the design of record (T2).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* llp: cross-link LLP 0246 into 0242, 0243, 0244 Related lines
The design of record for the proxy-mode-default-attach change set
(LLP 0246) landed after the request and decision docs it closes.
Append it to the Related metadata line of LLP 0242 (issue), LLP 0243
and LLP 0244 (decisions) so the corpus forward-links to the design,
per LLP 0156's allowance for trivial editorial forward-refs on
Accepted docs. Re-verified all six anchors LLP 0246 and the annotated
code cite (#composed-default, #user-key-wins in 0243;
#attach-offers, #enable-write, #central-managed, #non-interactive in
0244) resolve, and ran ref-check over llp/ plus every file carrying
an @ref LLP 0243/0244 annotation: zero broken references.
Task-Id: T2
* T1: pin --json attach never prompts even on a TTY (LLP 0244 #non-interactive)
test/core/attach-proxy-migration.test.js was missing coverage for the
--json attach shape: the code gates on parsed.json (clients.js line 886)
but no test exercised it. Add two cases riding the existing harness (its
json opt was plumbed but unused): --json on a TTY, and --json combined
with non-TTY, both asserting the askYesNo seam is never reached, stderr
carries exactly the one pointer line, no config write occurs, and stdout
stays the attach's valid JSON payload with nothing interleaved.
Task-Id: T1
* LLP 0246 renumbered to 0251: master claimed 0246 for remote-control-absolute-form-requests
The design was minted at 0246 while, concurrently, llp/0246-remote-control-absolute-form-requests.issue.md
merged to master. Two live docs at one number produce an ambiguous @ref and a red
ref-hygiene check once this branch merges, and neither branch's CI can see it.
Per the repo's renumber rule the later claimant moves above the highest number
claimed anywhere (0250), so the design becomes 0251. Reference sweep covers the
plan (LLP 0249), the design's own title, and the three corpus cross-links T2 added
to LLP 0242/0243/0244. No prose meaning changes.
* Review: make the --json attach pins fail loudly instead of parking on an unanswerable prompt
The new --json-on-a-TTY pin queued no stdin answer and leaned on askYesNo
hanging as its guard. Removing the parsed.json term from the clients.js gate
proved that inverts the guard: runAttach never settles, every assertion after
it (including the one that names the bug) is skipped, withTempHome's cleanup
never runs, and node reports cancelledByParent for the four tests behind it.
Queue a 'y' like the sibling dry-run and attach-all cases, so a reached seam
accepts the migration and trips the question, stderr, and config-unchanged
assertions in place: one clean red test, no cascade.
Also read the json flag off the attach args rather than the harness closure,
the way a real adapter reads it, so the same pins fail if the command stops
propagating --json into client.attach(). Verified both perturbations: gate
removal now fails test 7 alone with testCodeFailure; dropping json from the
attach args fails both new tests, where before this change both stayed green.
---------
Co-authored-by: test <test@test.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: test <test@example.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@philcunliffe