From b9ed8b2b155333a70fd29c9c16e6a58f1ca00362 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 13:54:10 -0700 Subject: [PATCH 01/15] Plan: client attach on join (LLP 0046, implements 0045) --- llp/0046-client-attach.plan.md | 203 +++++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 llp/0046-client-attach.plan.md diff --git a/llp/0046-client-attach.plan.md b/llp/0046-client-attach.plan.md new file mode 100644 index 0000000..52eb741 --- /dev/null +++ b/llp/0046-client-attach.plan.md @@ -0,0 +1,203 @@ +# LLP 0046: Client attach on join — implementation plan + +**Type:** plan +**Status:** Active +**Systems:** Config, Daemon, Onboarding, Sources, Gateway +**Author:** neutral +**Date:** 2026-06-26 +**Related:** 0044, 0045 +**Generated-by:** neutral + +> [LLP 0045](./0045-client-attach.design.md) is the implementation design for +> **client attach on join** — the reversible instance of the +> [LLP 0036](./0036-central-config-driven-client-actions.decision.md) action seam +> ([decision: LLP 0044](./0044-client-attach-on-join.decision.md)). It already +> names the files and functions to add or change and groups them into nine +> independently-mergeable seams. This plan turns those seams into a task graph +> with real code dependency edges, so the first wave can parallelize and each +> task merges on its own without leaving the tree broken. + +## How this refines the design + +The design's "Module / seam breakdown" lists nine modules. This plan keeps that +exact decomposition — it is already minimal and each seam is independently +testable — maps each task 1:1 onto the design's seam numbering for traceability, +and only makes the dependency edges explicit so neutral can schedule them. A +tenth task (T10) is the lifecycle move that marks 0045 shipped once the substance +has merged. + +The shape of the graph: + +- **Foundational / independent (deps `[]`).** Four tasks have no in-repo + dependency on each other and form the first wave: + - **T1 — `attach()` writes a self-describing undo record.** Adapter-local + edits to the claude and codex `attach()` paths so each marker carries + everything the single core undo (T4) needs to reverse *without the plugin + loaded* — claude's `_hypaware` marker + ([`hypaware-core/plugins-workspace/claude/src/settings.js`](../hypaware-core/plugins-workspace/claude/src/settings.js), + `MARKER_KEY`; surfaced via `writeAttachOutput` / + [`anthropic.js`](../hypaware-core/plugins-workspace/claude/src/anthropic.js)) + records `prev_base_url` plus the managed `ANTHROPIC_BASE_URL` / + `SessionStart` hook entries; codex's `# BEGIN/END hypaware` marked block + ([`hypaware-core/plugins-workspace/codex/src/settings.js`](../hypaware-core/plugins-workspace/codex/src/settings.js)) + records the prior `model_provider`. Unit-tested via the marker contents. + Independent — it touches only adapter files and changes no core contract + (the `json: true` one-line output shape is unchanged). + - **T2 — attach policy reader.** New + [`src/core/config/attach_policy.js`](../src/core/config/attach_policy.js) + (`readAttachPolicy`, tri-state over `config.attach.on_join`), the + [`backfill_policy.js`](../src/core/config/backfill_policy.js) twin — the + single source of truth shared by the reconciler handler (T6) and the status + surface (T9) so the two can never disagree on what a block means. Pure; + unit-tested. + - **T3 — reconcile-context seam.** Extend `ActionContext` and `ReconcileInput` + in [`src/core/config/types.d.ts`](../src/core/config/types.d.ts) with the + three optional fields the attach handler reads — + `clientDescriptors?: Map` (enumerate / + client→plugin map), `clients?: AiGatewayCapability` (invoke the effect), and + `endpoint?: string` (the local gateway base URL). Type-only; no behaviour + change until a handler (T6) or the daemon (T7) reads them. + - **T8 — per-plugin `attach` config validation.** A `validateAttachSection` + beside `validateBackfillSection` in the claude/codex + [`config.js`](../hypaware-core/plugins-workspace/claude/src/config.js), + wired into each plugin's config-section validator + (`validateClaudeConfig` / the codex equivalent). Validates + `config.attach.on_join`; plugin-local, no top-level/core schema change. It + only needs to land before the end-to-end smoke proves an opt-out, not before + any core code compiles. + +- **The single core undo (deps `[T1]`).** **T4 — the one detach + implementation.** New + [`src/core/config/client_detach_disk.js`](../src/core/config/client_detach_disk.js): + reverse a client's attach from the descriptor's `attachProbe` + the marker + undo record, **format-aware** (`json` marker-key / `toml` managed-block) but + plugin-agnostic, reusing `resolveClientSettingsPath` + ([`src/core/daemon/client_settings_path.js`](../src/core/daemon/client_settings_path.js)) + and the `probeClientAttached` format logic + ([`src/core/daemon/status.js`](../src/core/daemon/status.js)). It subsumes the + adapters' old `detach()` — including the Codex marked-block strip and prior + `model_provider` restore — so it replays exactly the undo record T1 writes + (hence the edge on T1). Unit-tested on fixture settings files with **no plugin + loaded**, proving reverse never depends on `ctx.clients`. + +- **Retire the old detach (deps `[T4]`).** **T5 — drop adapter `detach()` + + reroute manual `hyp detach`.** Remove `detach` from + `AiGatewayClientRegistration` + ([`collectivus-plugin-kernel-types.d.ts`](../collectivus-plugin-kernel-types.d.ts)) + and from the claude/codex `registerClient(...)` calls, and point + `runClientLifecycle`'s detach branch + ([`src/core/cli/core_commands.js`](../src/core/cli/core_commands.js)) at the T4 + core undo, resolved via the `clientDescriptor`. There is then exactly one undo + and it cannot drift from itself. The existing `claude_attach_detach` / + `client_attach_idempotent` smokes now exercise the core undo and must stay + green — they are the cross-format regression for the single undo. + +- **The attach handler (deps `[T2, T3, T4]`).** **T6 — `action_attach.js`.** New + [`src/core/config/action_attach.js`](../src/core/config/action_attach.js) + (`createAttachHandler` + `attachHandler`), mirroring `action_backfill.js`: + `desired()` iterates `ctx.clientDescriptors` ∩ enabled plugins ∩ + `readAttachPolicy` (T2), guarded on `ctx.clients.getClient(name)` being present + (T3); `perform()` calls `attach({ endpoint: ctx.endpoint, json: true })`, + parses the one-line JSON, and records the marker detail (`settings_path`, + `prev_value`); `reverse()` invokes the **T4 disk-driven undo** (it does *not* + call `ctx.clients`, which lacks the dropped client). Unit-tested with injected + fake `clientDescriptors` + `clients` + filesystem. + +- **Daemon wiring (deps `[T3, T6]`).** **T7 — `runtime.js`.** In + [`src/core/daemon/runtime.js`](../src/core/daemon/runtime.js): resolve + `clientDescriptors` (from the plugin catalog), `clients`/`endpoint` (from + `boot.runtime.capabilities` when the gateway capability is present, via + `gateway.localEndpoint()` with the `configuredGatewayEndpoint` fallback the CLI + already uses), thread all three onto the reconcile context (T3), and register + the handlers **`[attachHandler, backfillHandler]` — attach first** so + in-process live-capture wiring starts ahead of the (possibly multi-minute) + backfill subprocess. Needs both the context fields (T3) and the handler it + registers (T6). + +- **Status surface (deps `[T2]`).** **T9 — declared-attach targets.** + `buildClientActionsReport` + ([`src/core/daemon/status.js`](../src/core/daemon/status.js)) gains a + declared-attach-targets derivation symmetric to backfill's, reusing the + `clientDescriptors`-derived plugin set status already builds (`backfillPlugins`) + and `readAttachPolicy` (T2) for the interpretation: an enabled client plugin on + a joined host (`hasCentral`) with no marker renders `pending`, with + `attach.on_join: false` or on a non-joined host renders `n/a`, and a `done` + marker renders attached. A failed/pending attach must **not** flip `overall` to + `degraded`. It renders the generic `done`/`failed` `attach` markers (the + handler writes them) unchanged; it never runs a pass, so it depends only on the + shared policy reader. + +- **Lifecycle flip (deps on all substance).** **T10** flips + [`llp/0045-client-attach.design.md`](./0045-client-attach.design.md) Status + `Accepted → Active` — the shipped marker, landed only once the whole change set + has merged. + +This yields a 4-wide first wave (T1, T2, T3, T8), then T4 (behind T1), then a +fan-out of T5 (behind T4), T6 (behind T2/T3/T4) and T9 (behind T2), the single +integration task T7 (behind T3/T6), and finally the lifecycle flip T10. Each task +leaves the tree green: a richer-but-unread marker (T1), an unused policy reader +(T2), unread context fields (T3), accepted-but-unused config keys (T8), a core +undo with no second caller yet (T4), a defined-but-unregistered handler (T6), and +a status section that reads `pending`/`n/a` until a pass runs (T9) are all inert +until T5 (reroute) and T7 (register + wire) connect them. + +## Test ownership per task + +- **T1:** the claude `_hypaware` marker and codex marked block each carry a + complete undo record (claude: `prev_base_url` + managed hook entries; codex: + prior `model_provider`); an idempotent re-attach keeps the *original* + `prev_base_url`, not the gateway URL. +- **T2:** `readAttachPolicy` tri-state — absent block → default-on + (`onJoin: undefined`), `on_join: false` → opt-out, present-but-malformed → + fail-safe opt-out (mirroring `backfill_policy.js`). +- **T3:** type-only; covered by the type-check / build and by T6's handler tests + reading the new context fields. +- **T4:** disk-driven reverse with **no adapter loaded** — given a fixture + settings file with a hypaware marker, the generic undo strips the managed + keys/hooks/block, restores `prev_base_url` / prior `model_provider`, and leaves + no orphaned `hyp claude-hook` entries. A pre-existing foreign base URL + round-trips byte-for-byte; the no-pre-existing-URL fixture round-trips to empty. +- **T5:** one undo, both call sites — the core undo is exercised by manual + `hyp detach` *and* (in T6) the reconciler `reverse()` against the same fixtures + and reaches the same end-state; the `claude_attach_detach` / + `client_attach_idempotent` smokes stay green. +- **T6:** reverse gap — a `desired()` that drops a previously-applied client + triggers `reverse()` once (invoking the T4 undo), the marker is removed, and a + second pass is a no-op; a throwing `attach()` writes a `failed` marker that + retries with bumped `attempts`; daemon-only — with no gateway capability + `clients`/`endpoint` are undefined and the handler is inert. +- **T7:** the daemon resolves `clientDescriptors`/`clients`/`endpoint` and + registers `[attachHandler, backfillHandler]` (attach first); a boot + already-confirmed pass and a confirm-edge pass both reach the handler. +- **T8:** the claude/codex section validator accepts `{ on_join }` and rejects a + non-boolean; a central-locked `attach` entry cannot be flipped by a colliding + local entry (reuse the LLP 0031 merge-drop harness). +- **T9:** mixed `done`/`failed`/`pending`/`n/a` reads cleanly; `attach.on_join: + false` and a non-joined host both render `n/a`; a `failed` attach does not flip + `overall` to `degraded`. +- **End-to-end (hermetic smoke, lands with T7):** a seeded join confirming a + config that names `@hypaware/claude` auto-attaches (settings written, marker + `done`, status attached) and does not re-attach on a second confirmed poll; + then a follow-up confirmed config that **drops** `@hypaware/claude` reverses it + (settings restored) — the Part 5 config-drop trigger, exercised post-restart. + +## Tasks +- id: T1 branch: task/client-attach/T1 deps: [] -- attach() writes a self-describing undo record: claude `_hypaware` marker (claude/src/settings.js MARKER_KEY, via writeAttachOutput/anthropic.js) records prev_base_url + managed ANTHROPIC_BASE_URL/SessionStart hook entries; codex `# BEGIN/END hypaware` marked block (codex/src/settings.js) records prior model_provider. Unit-tested via marker contents. +- id: T2 branch: task/client-attach/T2 deps: [] -- Attach policy reader: new src/core/config/attach_policy.js (readAttachPolicy, tri-state over config.attach.on_join), the backfill_policy.js twin shared by the handler (T6) and status (T9). Pure; unit-tested. +- id: T3 branch: task/client-attach/T3 deps: [] -- Reconcile-context seam: extend ActionContext + ReconcileInput in src/core/config/types.d.ts with optional clientDescriptors (Map), clients (AiGatewayCapability), endpoint (string). Type-only; no behaviour change until a handler reads them. +- id: T8 branch: task/client-attach/T8 deps: [] -- Per-plugin attach config validation: validateAttachSection beside validateBackfillSection in the claude/codex config.js, wired into validateClaudeConfig and the codex equivalent. Validates config.attach.on_join; plugin-local, no core schema change. +- id: T4 branch: task/client-attach/T4 deps: [T1] -- The single core undo (= detach): new src/core/config/client_detach_disk.js reversing a client's attach from the attachProbe descriptor + the marker undo record, format-aware (json marker-key / toml managed-block) but plugin-agnostic, reusing resolveClientSettingsPath (client_settings_path.js) and the probeClientAttached format logic (status.js). Subsumes the adapters' old detach() incl. the Codex marked-block strip + model_provider restore. Unit-tested on fixtures with no plugin loaded. +- id: T5 branch: task/client-attach/T5 deps: [T4] -- Retire adapter detach() + reroute manual detach: drop detach from AiGatewayClientRegistration (collectivus-plugin-kernel-types.d.ts) and from the claude/codex registerClient() calls, and point runClientLifecycle's detach branch (src/core/cli/core_commands.js) at the T4 core undo via the clientDescriptor. The claude_attach_detach / client_attach_idempotent smokes must stay green. +- id: T6 branch: task/client-attach/T6 deps: [T2, T3, T4] -- Attach handler: new src/core/config/action_attach.js (createAttachHandler + attachHandler) mirroring action_backfill.js. desired() over ctx.clientDescriptors ∩ enabled plugins ∩ readAttachPolicy, guarded on ctx.clients.getClient(name); perform() calls attach({ endpoint: ctx.endpoint, json:true }), parses the one-line JSON, records detail (settings_path, prev_value); reverse() invokes the T4 disk-driven undo (not ctx.clients). Unit-tested with injected fake clientDescriptors + clients + fs. +- id: T7 branch: task/client-attach/T7 deps: [T3, T6] -- Daemon wiring in src/core/daemon/runtime.js: resolve clientDescriptors (plugin catalog), clients/endpoint (boot.runtime.capabilities when the gateway is enabled, gateway.localEndpoint() with the configuredGatewayEndpoint fallback), thread onto the reconcile context, and register handlers [attachHandler, backfillHandler] — attach first. Lands the end-to-end hermetic auto-attach/reverse smoke. +- id: T9 branch: task/client-attach/T9 deps: [T2] -- Status surface: buildClientActionsReport (src/core/daemon/status.js) gains a declared-attach-targets derivation symmetric to backfill's, reusing the clientDescriptors-derived plugin set (backfillPlugins) and readAttachPolicy: enabled client plugin on a joined host -> pending (no marker) / n-a (on_join:false or non-joined) / attached (done marker); must not flip overall to degraded. +- id: T10 branch: task/client-attach/T10 deps: [T1, T2, T3, T4, T5, T6, T7, T8, T9] -- flip LLP 0045 Status Accepted->Active (shipped marker, LLP 0016). + +## References + +- [LLP 0045](./0045-client-attach.design.md) — the implementation design this plan schedules +- [LLP 0044](./0044-client-attach-on-join.decision.md) — client attach on join (the decision) +- [LLP 0041](./0041-central-config-client-actions.design.md) — the seam + backfill design this mirrors (sibling plan: [LLP 0043](./0043-central-config-client-actions.plan.md)) +- [LLP 0036](./0036-central-config-driven-client-actions.decision.md) — the action seam (reversible-handler contract) +- [LLP 0016](./0016-ai-gateway.decision.md) — AI gateway / client adapters (`registerClient`, `attach`, `localEndpoint`) +- [`src/core/config/action_reconciler.js`](../src/core/config/action_reconciler.js), [`src/core/config/action_backfill.js`](../src/core/config/action_backfill.js), [`src/core/config/backfill_policy.js`](../src/core/config/backfill_policy.js), [`src/core/daemon/runtime.js`](../src/core/daemon/runtime.js), [`src/core/daemon/status.js`](../src/core/daemon/status.js), [`src/core/daemon/client_settings_path.js`](../src/core/daemon/client_settings_path.js), [`src/core/cli/core_commands.js`](../src/core/cli/core_commands.js), [`src/core/plugin_catalog.js`](../src/core/plugin_catalog.js) — the code these tasks build on From 330bc41a45ccc93469c3325388cd00c2ad7d4782 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:07:28 -0700 Subject: [PATCH 02/15] Attach policy reader: tri-state readAttachPolicy (LLP 0044/0045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New src/core/config/attach_policy.js — the backfill_policy.js twin and single source of truth for interpreting a client adapter plugin entry's `config.attach.on_join` block, shared by the attach handler (T6) and the declared-attach status surface (T9) so the two can never disagree. Tri-state, mirroring readBackfillPolicy: - absent block → default-on (onJoin: undefined) - on_join: true/false → that value - present-but-malformed (non-boolean on_join) → fail-safe opt-out (false); silently editing a user-owned client settings file on a malformed opt-out is the wrong default, so do not fail open. Pure; unit-tested over the full tri-state plus non-object attach blocks and the `onJoin !== false` off-switch contract both consumers rely on. @ref LLP 0044#where-attach-is-declared @ref LLP 0045 Task-Id: T2 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/config/attach_policy.js | 54 +++++++++++++++++++ test/core/attach-policy.test.js | 93 ++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 src/core/config/attach_policy.js create mode 100644 test/core/attach-policy.test.js diff --git a/src/core/config/attach_policy.js b/src/core/config/attach_policy.js new file mode 100644 index 0000000..029ee38 --- /dev/null +++ b/src/core/config/attach_policy.js @@ -0,0 +1,54 @@ +// @ts-check + +/** + * @import { PluginConfigInstance } from '../../../collectivus-plugin-kernel-types.d.ts' + */ + +/** + * Read a client adapter plugin entry's `attach` policy block (LLP 0044) as a + * tri-state. The single source of truth for interpreting the block — shared + * by the reconciler (`action_attach.js`, which decides whether to attach a + * client on join) and the status surface (`status.js`, which renders the + * declared-attach `pending`/`n/a` derivation) so the two can never disagree + * on what a given block means. The `backfill_policy.js` twin. + * + * A *missing* block is the default (on_join on → `onJoin: undefined`): the + * reconcile path must not throw on a config the plugin validator (LLP 0044) + * already accepted, and an enabled client adapter on a joined host attaches by + * default ([LLP 0044 §Consent](../../../llp/0044-client-attach-on-join.decision.md)). + * + * A block that is *present but malformed* must not fail open, though: a + * non-boolean `on_join` (e.g. the JSON typo `on_join: "false"`) is treated as + * an opt-out (`onJoin: false`), never as "default on". The operator clearly + * intended to set the flag, and silently editing a user-owned client settings + * file is the wrong thing to do on a malformed opt-out. (With the per-plugin + * validator live — T8 — such a config is rejected at apply time anyway; this + * is the belt-and-braces read both consumers share.) + * + * Consumers test the off switch as `readAttachPolicy(entry).onJoin !== false`, + * so both the default (`undefined`) and an explicit `true` mean "attach". + * + * @param {PluginConfigInstance | undefined} entry + * @returns {{ onJoin: boolean | undefined }} + * @ref LLP 0044#where-attach-is-declared [constrained-by] — attach policy (`on_join`) is owned by the client plugin; the kernel only reads it + */ +export function readAttachPolicy(entry) { + const config = entry?.config + const attach = + config && typeof config === 'object' && !Array.isArray(config) + ? /** @type {Record} */ (config).attach + : undefined + if (!attach || typeof attach !== 'object' || Array.isArray(attach)) { + return { onJoin: undefined } + } + const raw = /** @type {Record} */ (attach) + // Absent → default on (undefined). Present-and-boolean → that value. + // Present-but-non-boolean → opt-out (false): do not fail open. + const onJoin = + raw.on_join === undefined + ? undefined + : typeof raw.on_join === 'boolean' + ? raw.on_join + : false + return { onJoin } +} diff --git a/test/core/attach-policy.test.js b/test/core/attach-policy.test.js new file mode 100644 index 0000000..21e7aed --- /dev/null +++ b/test/core/attach-policy.test.js @@ -0,0 +1,93 @@ +// @ts-check + +import test from 'node:test' +import assert from 'node:assert/strict' + +import { readAttachPolicy } from '../../src/core/config/attach_policy.js' + +/** + * @import { PluginConfigInstance } from '../../collectivus-plugin-kernel-types.d.ts' + */ + +/** + * Build a minimal client adapter plugin entry whose `config.attach` block is + * exactly `attach`. + * @param {unknown} attach + * @returns {PluginConfigInstance} + */ +function entryWithAttach(attach) { + return /** @type {PluginConfigInstance} */ ({ + name: '@hypaware/claude', + config: { attach }, + }) +} + +test('readAttachPolicy: no entry → default-on (onJoin undefined)', () => { + assert.deepEqual(readAttachPolicy(undefined), { onJoin: undefined }) +}) + +test('readAttachPolicy: entry with no config → default-on', () => { + const entry = /** @type {PluginConfigInstance} */ ({ name: '@hypaware/claude' }) + assert.deepEqual(readAttachPolicy(entry), { onJoin: undefined }) +}) + +test('readAttachPolicy: absent attach block → default-on (onJoin undefined)', () => { + const entry = /** @type {PluginConfigInstance} */ ({ + name: '@hypaware/claude', + config: { proxy: '@hypaware/ai-gateway' }, + }) + assert.deepEqual(readAttachPolicy(entry), { onJoin: undefined }) +}) + +test('readAttachPolicy: present block with no on_join → default-on', () => { + assert.deepEqual(readAttachPolicy(entryWithAttach({})), { onJoin: undefined }) +}) + +test('readAttachPolicy: on_join: true → opt-in', () => { + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: true })), { + onJoin: true, + }) +}) + +test('readAttachPolicy: on_join: false → opt-out', () => { + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: false })), { + onJoin: false, + }) +}) + +test('readAttachPolicy: present-but-malformed on_join → fail-safe opt-out', () => { + // The JSON-typo case: a string "false" is not "default on"; do not fail open. + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: 'false' })), { + onJoin: false, + }) + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: 'true' })), { + onJoin: false, + }) + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: 1 })), { + onJoin: false, + }) + assert.deepEqual(readAttachPolicy(entryWithAttach({ on_join: null })), { + onJoin: false, + }) +}) + +test('readAttachPolicy: attach block that is an array → default-on (ignored)', () => { + assert.deepEqual(readAttachPolicy(entryWithAttach(['on_join'])), { + onJoin: undefined, + }) +}) + +test('readAttachPolicy: attach block that is a scalar → default-on (ignored)', () => { + assert.deepEqual(readAttachPolicy(entryWithAttach('true')), { + onJoin: undefined, + }) +}) + +test('readAttachPolicy: off switch reads as onJoin !== false', () => { + // The contract both consumers (handler T6 / status T9) rely on: default and + // explicit-true both mean "attach"; only an explicit/coerced false opts out. + assert.equal(readAttachPolicy(undefined).onJoin !== false, true) + assert.equal(readAttachPolicy(entryWithAttach({ on_join: true })).onJoin !== false, true) + assert.equal(readAttachPolicy(entryWithAttach({ on_join: false })).onJoin !== false, false) + assert.equal(readAttachPolicy(entryWithAttach({ on_join: 'false' })).onJoin !== false, false) +}) From 7e8b728b3c2725172f9190303a967972ae4c306c Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:07:32 -0700 Subject: [PATCH 03/15] Client attach: reconcile-context seam (clientDescriptors/clients/endpoint) (LLP 0045) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend ActionContext and ReconcileInput in src/core/config/types.d.ts with the three optional fields the attach handler reads, mirroring LLP 0045 §Part 1: - clientDescriptors?: Map — the static client→plugin map (from buildPluginCatalog) used to enumerate desired() and to hand the disk-driven undo each descriptor's attachProbe. - clients?: AiGatewayCapability — runtime gateway capability used only to invoke a client's attach effect (getClient(name).attach). - endpoint?: string — the local gateway base URL clients attach to. All optional and daemon-only by construction (a plain CLI boot leaves them unset). Type-only; no behaviour change until a handler (T6) or the daemon (T7) reads them. Imports AiGatewayCapability from the kernel types and ClientDescriptor from plugin_catalog.js. typecheck, lint, and the full test suite (1455 pass) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) Task-Id: T3 --- src/core/config/types.d.ts | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/core/config/types.d.ts b/src/core/config/types.d.ts index 3160f80..791fbb0 100644 --- a/src/core/config/types.d.ts +++ b/src/core/config/types.d.ts @@ -11,7 +11,9 @@ import type { BackfillRegistry, PluginLogger, JsonObject, + AiGatewayCapability, } from '../../../collectivus-plugin-kernel-types.d.ts' +import type { ClientDescriptor } from '../plugin_catalog.js' /** * Outcome of the `init` overwrite guard (LLP 0031). `proceed` is true @@ -374,6 +376,29 @@ export interface ActionContext { * `process.env.HYP_HOME` happened to be (LLP 0041 §Run-once flow step 2). */ env: NodeJS.ProcessEnv + /** + * Static client→plugin map (`clientName -> { plugin, name, attachProbe? }`) + * derived from manifests by `buildPluginCatalog`. The attach handler + * enumerates `desired()` off this map — the runtime `clients` registry + * carries no owning-plugin field, so descriptors are the source of truth + * for "is this client's plugin enabled?" and hand the disk-driven undo the + * `attachProbe` it replays from (LLP 0045 §Part 1, §Part 3). Daemon-only — + * a plain CLI boot leaves it unset and any client handler stays inert. + */ + clientDescriptors?: Map + /** + * Runtime gateway capability, used only to *invoke* a client's effect + * (`getClient(name).attach(...)`). Present when the AI gateway plugin is + * enabled; `desired()` guards on `getClient(name)` so it never names a + * client `perform()` cannot reach (LLP 0045 §Part 1). + */ + clients?: AiGatewayCapability + /** + * The local gateway base URL clients attach to, resolved from + * `gateway.localEndpoint()` with the configured-`listen` fallback the CLI + * uses. Set whenever `clients` is (LLP 0045 §Part 1). + */ + endpoint?: string /** Injectable clock (test seam). */ now: () => number log: PluginLogger @@ -414,6 +439,23 @@ export interface ReconcileInput { * diverge from `process.env` (the direct-`runDaemon`/hermetic-smoke path). */ env: NodeJS.ProcessEnv + /** + * Static client→plugin map the daemon resolves from the plugin catalog and + * threads onto {@link ActionContext} so a client handler can enumerate + * `desired()` and read each descriptor's `attachProbe` (LLP 0045 §Part 1). + * Absent on a plain CLI boot. + */ + clientDescriptors?: Map + /** + * Runtime gateway capability for invoking a client's attach effect, present + * when the AI gateway plugin is enabled (LLP 0045 §Part 1). + */ + clients?: AiGatewayCapability + /** + * The local gateway base URL clients attach to; set whenever `clients` is + * (LLP 0045 §Part 1). + */ + endpoint?: string } /** What the reconciler did with one (handler, requestKey) unit on a pass. */ From 949768bbe502ec904a68eb6469b6b49c5c4296ea Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:09:30 -0700 Subject: [PATCH 04/15] Per-plugin attach config validation: validateAttachSection (LLP 0045 T8) Add validateAttachSection beside validateBackfillSection in the claude and codex config.js, wired into validateClaudeConfig / validateCodexConfig. It validates the optional config.attach.on_join (boolean, default true) and rejects unknown attach keys, mirroring the backfill section validator. Plugin-local and additive: no top-level/core schema change. Realizes LLP 0045 Part 4 (attach.on_join rides the client adapter's own config block, validated by that plugin's config-section validator). Inert until the attach handler (T6) and daemon wiring (T7) read the policy. Tests cover accept/reject for both plugins, pointer mounting, and the LLP 0031 central-lock merge-drop (a local attach.on_join cannot flip a central-locked entry). Task-Id: T8 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../plugins-workspace/claude/src/config.js | 58 ++++++++++++++--- .../plugins-workspace/codex/src/config.js | 57 ++++++++++++++--- test/plugins/claude-config.test.js | 62 +++++++++++++++++++ test/plugins/codex-config.test.js | 37 +++++++++++ 4 files changed, 199 insertions(+), 15 deletions(-) diff --git a/hypaware-core/plugins-workspace/claude/src/config.js b/hypaware-core/plugins-workspace/claude/src/config.js index a76c277..a219fd5 100644 --- a/hypaware-core/plugins-workspace/claude/src/config.js +++ b/hypaware-core/plugins-workspace/claude/src/config.js @@ -2,11 +2,12 @@ /** * Config validation for the `@hypaware/claude` plugin's own `config` - * block. v1 validates only the optional `backfill` sub-object that drives - * backfill-on-join — `{ on_join, window_days }`. Every other key (e.g. - * `proxy`) passes through untouched so existing configs keep working; - * there is no top-level `backfill` section and nothing new for core to - * validate. + * block. v1 validates the optional `backfill` sub-object that drives + * backfill-on-join — `{ on_join, window_days }` — and the optional + * `attach` sub-object that drives attach-on-join — `{ on_join }`. Every + * other key (e.g. `proxy`) passes through untouched so existing configs + * keep working; there is no top-level `backfill`/`attach` section and + * nothing new for core to validate. * * Pure and dependency-free: it returns a `ValidationResult` so it plugs * straight into `ctx.configRegistry.registerSection` and is callable from @@ -20,8 +21,9 @@ export const CLAUDE_CONFIG_SECTION = 'claude' /** * Validate the `@hypaware/claude` plugin config slice. Only the optional - * `backfill` policy block is checked; unknown sibling keys are ignored so - * the validator stays additive over the existing config surface. + * `backfill` and `attach` policy blocks are checked; unknown sibling keys + * are ignored so the validator stays additive over the existing config + * surface. * * @ref LLP 0037#per-plugin-config-kernel-generic-reconciler [implements] — * backfill policy ({ on_join, window_days }) lives in and is validated @@ -37,7 +39,10 @@ export function validateClaudeConfig(value) { return { ok: false, errors: [{ pointer: '', message: 'claude config must be an object' }] } } const raw = /** @type {Record} */ (value) - const errors = validateBackfillSection(raw.backfill, '/backfill') + const errors = [ + ...validateBackfillSection(raw.backfill, '/backfill'), + ...validateAttachSection(raw.attach, '/attach'), + ] if (errors.length > 0) return { ok: false, errors } return { ok: true } } @@ -82,3 +87,40 @@ export function validateBackfillSection(value, pointer) { } return errors } + +/** + * Validate the optional `attach` policy block on a client-adapter plugin's + * config: `on_join` (whether the daemon auto-attaches this client when a + * joined host confirms a central config that enables it, boolean, + * default true). Optional; unknown keys are rejected so a typo + * (`on_joins`) surfaces instead of being silently ignored. Pure — the + * caller chooses where the returned pointers mount. + * + * @ref LLP 0045#part-4--per-plugin-attach-config--status-surface [implements] — + * attach.on_join rides the client adapter's own config block, validated + * by this plugin's config-section validator beside validateBackfillSection; + * no top-level/core schema. + * + * @param {unknown} value + * @param {string} pointer JSON-pointer prefix for the `attach` object + * @returns {ValidationError[]} + */ +export function validateAttachSection(value, pointer) { + /** @type {ValidationError[]} */ + const errors = [] + if (value === undefined) return errors + if (value === null || typeof value !== 'object' || Array.isArray(value)) { + errors.push({ pointer, message: 'attach must be an object' }) + return errors + } + const raw = /** @type {Record} */ (value) + if (raw.on_join !== undefined && typeof raw.on_join !== 'boolean') { + errors.push({ pointer: `${pointer}/on_join`, message: 'attach.on_join must be a boolean' }) + } + for (const key of Object.keys(raw)) { + if (key !== 'on_join') { + errors.push({ pointer: `${pointer}/${key}`, message: `unknown attach key '${key}'` }) + } + } + return errors +} diff --git a/hypaware-core/plugins-workspace/codex/src/config.js b/hypaware-core/plugins-workspace/codex/src/config.js index 215ce52..781f7f4 100644 --- a/hypaware-core/plugins-workspace/codex/src/config.js +++ b/hypaware-core/plugins-workspace/codex/src/config.js @@ -2,10 +2,12 @@ /** * Config validation for the `@hypaware/codex` plugin's own `config` - * block. v1 validates only the optional `backfill` sub-object that drives - * backfill-on-join — `{ on_join, window_days }`. Every other key passes - * through untouched so existing configs keep working; there is no - * top-level `backfill` section and nothing new for core to validate. + * block. v1 validates the optional `backfill` sub-object that drives + * backfill-on-join — `{ on_join, window_days }` — and the optional + * `attach` sub-object that drives attach-on-join — `{ on_join }`. Every + * other key passes through untouched so existing configs keep working; + * there is no top-level `backfill`/`attach` section and nothing new for + * core to validate. * * Pure and dependency-free: it returns a `ValidationResult` so it plugs * straight into `ctx.configRegistry.registerSection` and is callable from @@ -19,8 +21,9 @@ export const CODEX_CONFIG_SECTION = 'codex' /** * Validate the `@hypaware/codex` plugin config slice. Only the optional - * `backfill` policy block is checked; unknown sibling keys are ignored so - * the validator stays additive over the existing config surface. + * `backfill` and `attach` policy blocks are checked; unknown sibling keys + * are ignored so the validator stays additive over the existing config + * surface. * * @ref LLP 0037#per-plugin-config-kernel-generic-reconciler [implements] — * backfill policy ({ on_join, window_days }) lives in and is validated @@ -36,7 +39,10 @@ export function validateCodexConfig(value) { return { ok: false, errors: [{ pointer: '', message: 'codex config must be an object' }] } } const raw = /** @type {Record} */ (value) - const errors = validateBackfillSection(raw.backfill, '/backfill') + const errors = [ + ...validateBackfillSection(raw.backfill, '/backfill'), + ...validateAttachSection(raw.attach, '/attach'), + ] if (errors.length > 0) return { ok: false, errors } return { ok: true } } @@ -81,3 +87,40 @@ export function validateBackfillSection(value, pointer) { } return errors } + +/** + * Validate the optional `attach` policy block on a client-adapter plugin's + * config: `on_join` (whether the daemon auto-attaches this client when a + * joined host confirms a central config that enables it, boolean, + * default true). Optional; unknown keys are rejected so a typo + * (`on_joins`) surfaces instead of being silently ignored. Pure — the + * caller chooses where the returned pointers mount. + * + * @ref LLP 0045#part-4--per-plugin-attach-config--status-surface [implements] — + * attach.on_join rides the client adapter's own config block, validated + * by this plugin's config-section validator beside validateBackfillSection; + * no top-level/core schema. + * + * @param {unknown} value + * @param {string} pointer JSON-pointer prefix for the `attach` object + * @returns {ValidationError[]} + */ +export function validateAttachSection(value, pointer) { + /** @type {ValidationError[]} */ + const errors = [] + if (value === undefined) return errors + if (value === null || typeof value !== 'object' || Array.isArray(value)) { + errors.push({ pointer, message: 'attach must be an object' }) + return errors + } + const raw = /** @type {Record} */ (value) + if (raw.on_join !== undefined && typeof raw.on_join !== 'boolean') { + errors.push({ pointer: `${pointer}/on_join`, message: 'attach.on_join must be a boolean' }) + } + for (const key of Object.keys(raw)) { + if (key !== 'on_join') { + errors.push({ pointer: `${pointer}/${key}`, message: `unknown attach key '${key}'` }) + } + } + return errors +} diff --git a/test/plugins/claude-config.test.js b/test/plugins/claude-config.test.js index fe5c21f..7718327 100644 --- a/test/plugins/claude-config.test.js +++ b/test/plugins/claude-config.test.js @@ -5,6 +5,7 @@ import assert from 'node:assert/strict' import { CLAUDE_CONFIG_SECTION, + validateAttachSection, validateBackfillSection, validateClaudeConfig, } from '../../hypaware-core/plugins-workspace/claude/src/config.js' @@ -64,6 +65,42 @@ test('validateBackfillSection mounts pointers under the supplied prefix', () => assert.equal(errors[0].pointer, '/plugins/0/config/backfill/on_join') }) +test('validateClaudeConfig accepts an attach block', () => { + assert.deepEqual(validateClaudeConfig({ attach: { on_join: true } }), { ok: true }) + assert.deepEqual(validateClaudeConfig({ attach: { on_join: false } }), { ok: true }) + assert.deepEqual(validateClaudeConfig({ attach: {} }), { ok: true }) + assert.deepEqual( + validateClaudeConfig({ proxy: '@hypaware/ai-gateway', backfill: { on_join: true }, attach: { on_join: false } }), + { ok: true } + ) +}) + +test('validateClaudeConfig rejects a malformed attach block', () => { + /** @type {Array<[unknown, string]>} */ + const cases = [ + [{ attach: [] }, '/attach'], + [{ attach: 7 }, '/attach'], + [{ attach: null }, '/attach'], + [{ attach: { on_join: 'yes' } }, '/attach/on_join'], + [{ attach: { on_join: 1 } }, '/attach/on_join'], + [{ attach: { window_days: 30 } }, '/attach/window_days'], + [{ attach: { on_joins: true } }, '/attach/on_joins'], + ] + for (const [config, pointer] of cases) { + const result = validateClaudeConfig(config) + assert.equal(result.ok, false, `${JSON.stringify(config)} must fail`) + if (result.ok) continue + assert.equal(result.errors[0].pointer, pointer, `${JSON.stringify(config)} pointer`) + } +}) + +test('validateAttachSection mounts pointers under the supplied prefix', () => { + assert.deepEqual(validateAttachSection(undefined, '/attach'), []) + const errors = validateAttachSection({ on_join: 1 }, '/plugins/0/config/attach') + assert.equal(errors.length, 1) + assert.equal(errors[0].pointer, '/plugins/0/config/attach/on_join') +}) + test('the registered claude section drives validatePluginConfig', () => { const registry = createConfigRegistry() registry.registerSection({ @@ -102,3 +139,28 @@ test('a central-locked backfill.on_join cannot be flipped by a colliding local e { section: 'plugins', key: '@hypaware/claude', reason: 'collides_with_central' }, ]) }) + +test('a central-locked attach.on_join cannot be flipped by a colliding local entry', () => { + // LLP 0031 merge model + LLP 0044 opt-out (operator-only, no local + // override): a local entry that collides with a central-named plugin is + // dropped, so the operator's `attach.on_join: false` survives and the + // user cannot re-enable auto-attach locally. + const central = { + version: 2, + plugins: [{ name: '@hypaware/claude', config: { attach: { on_join: false } } }], + } + const local = { + version: 2, + plugins: [{ name: '@hypaware/claude', config: { attach: { on_join: true } } }], + } + const merged = mergeConfigLayers(/** @type {any} */ (central), /** @type {any} */ (local)) + + assert.equal(merged.effective.plugins?.length, 1) + assert.deepEqual( + /** @type {any} */ (merged.effective.plugins?.[0].config).attach, + { on_join: false } + ) + assert.deepEqual(merged.drops, [ + { section: 'plugins', key: '@hypaware/claude', reason: 'collides_with_central' }, + ]) +}) diff --git a/test/plugins/codex-config.test.js b/test/plugins/codex-config.test.js index 86eba39..6570938 100644 --- a/test/plugins/codex-config.test.js +++ b/test/plugins/codex-config.test.js @@ -5,6 +5,7 @@ import assert from 'node:assert/strict' import { CODEX_CONFIG_SECTION, + validateAttachSection, validateBackfillSection, validateCodexConfig, } from '../../hypaware-core/plugins-workspace/codex/src/config.js' @@ -57,6 +58,42 @@ test('validateBackfillSection mounts pointers under the supplied prefix', () => assert.equal(errors[0].pointer, '/plugins/0/config/backfill/window_days') }) +test('validateCodexConfig accepts an attach block', () => { + assert.deepEqual(validateCodexConfig({ attach: { on_join: true } }), { ok: true }) + assert.deepEqual(validateCodexConfig({ attach: { on_join: false } }), { ok: true }) + assert.deepEqual(validateCodexConfig({ attach: {} }), { ok: true }) + assert.deepEqual( + validateCodexConfig({ backfill: { on_join: true }, attach: { on_join: false } }), + { ok: true } + ) +}) + +test('validateCodexConfig rejects a malformed attach block', () => { + /** @type {Array<[unknown, string]>} */ + const cases = [ + [{ attach: [] }, '/attach'], + [{ attach: 42 }, '/attach'], + [{ attach: null }, '/attach'], + [{ attach: { on_join: 'yes' } }, '/attach/on_join'], + [{ attach: { on_join: 0 } }, '/attach/on_join'], + [{ attach: { window_days: 7 } }, '/attach/window_days'], + [{ attach: { on_joins: true } }, '/attach/on_joins'], + ] + for (const [config, pointer] of cases) { + const result = validateCodexConfig(config) + assert.equal(result.ok, false, `${JSON.stringify(config)} must fail`) + if (result.ok) continue + assert.equal(result.errors[0].pointer, pointer, `${JSON.stringify(config)} pointer`) + } +}) + +test('validateAttachSection mounts pointers under the supplied prefix', () => { + assert.deepEqual(validateAttachSection(undefined, '/attach'), []) + const errors = validateAttachSection({ on_join: 'no' }, '/plugins/0/config/attach') + assert.equal(errors.length, 1) + assert.equal(errors[0].pointer, '/plugins/0/config/attach/on_join') +}) + test('the registered codex section drives validatePluginConfig', () => { const registry = createConfigRegistry() registry.registerSection({ From 3cf01a13a325ac8d2f7f74896540a6b59718c141 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:13:50 -0700 Subject: [PATCH 05/15] T1: attach() writes a self-describing undo record (LLP 0045/0046) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make each client adapter's attach marker carry everything the single core undo (task 4) needs to reverse the attach from disk alone, with the plugin unloaded. claude (`_hypaware` marker): record `prev_base_url` (the restore target) plus the managed `env.ANTHROPIC_BASE_URL` and session-context hook entries. An idempotent re-attach preserves the marker's recorded original `prev_base_url` instead of backing up our own gateway URL over it, and reports that original via the attach result. codex (`# BEGIN/END hypaware` marked block): already self-delimiting and already records the prior `model_provider` as `# previous_model_provider` — locked in with tests (block is self-describing; re-attach keeps the original pointer). Unit-tested via the marker contents (no plugin/gateway loaded). Adapter-local; no core contract change (the `json: true` one-line attach output shape is unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) Task-Id: T1 --- .../plugins-workspace/claude/src/settings.js | 57 ++++++- test/plugins/claude-settings-attach.test.js | 152 ++++++++++++++++++ test/plugins/codex-toml-config.test.js | 29 ++++ 3 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 test/plugins/claude-settings-attach.test.js diff --git a/hypaware-core/plugins-workspace/claude/src/settings.js b/hypaware-core/plugins-workspace/claude/src/settings.js index 33ec2e5..fb14224 100644 --- a/hypaware-core/plugins-workspace/claude/src/settings.js +++ b/hypaware-core/plugins-workspace/claude/src/settings.js @@ -15,6 +15,13 @@ import path from 'node:path' * concurrent edit is detected instead of silently overwritten. The * `_hypaware` marker is what `detach()` keys off to know which keys * it inserted and is safe to remove. + * + * The marker is also a **self-describing undo record**: it records + * `prev_base_url` (the restore target) and the managed + * `env.ANTHROPIC_BASE_URL` / session-context hook entries it added, so + * a format-aware but plugin-agnostic core routine can reverse the + * attach from disk alone — with the plugin unloaded. See LLP 0045 + * Part 3. */ /** @@ -73,25 +80,46 @@ export async function attach(opts) { validateStateFile(stateFile) const { value, mtimeMs } = await readSettings(settingsPath) + const priorMarker = isPlainObject(value[MARKER_KEY]) ? value[MARKER_KEY] : undefined const env = ensureObject(value, 'env') - const previous = env.ANTHROPIC_BASE_URL - const prevValue = typeof previous === 'string' ? previous : undefined - - env.ANTHROPIC_BASE_URL = `http://127.0.0.1:${port}` - installSessionContextHooks(value, managedHookCommand(binPath, stateFile)) + const liveBaseUrl = typeof env.ANTHROPIC_BASE_URL === 'string' ? env.ANTHROPIC_BASE_URL : undefined + // Preserve the recorded original across a re-attach: once we own the + // URL the live value is *our* gateway URL, so keep the marker's + // recorded `prev_base_url` rather than backing up the gateway URL + // over it. A first attach backs up whatever was live. + // @ref LLP 0044#conflict--back-up--override-restore-on-leave [constrained-by] — the marker IS the backup restored on leave + const prevBaseUrl = priorMarker + ? (typeof priorMarker.prev_base_url === 'string' ? priorMarker.prev_base_url : undefined) + : liveBaseUrl + + const baseUrl = `http://127.0.0.1:${port}` + const command = managedHookCommand(binPath, stateFile) + + env.ANTHROPIC_BASE_URL = baseUrl + installSessionContextHooks(value, command) + // Self-describing undo record: enough for the format-aware core undo + // to restore-or-remove `env.ANTHROPIC_BASE_URL`, strip the managed + // hook entries, and delete the marker without loading this plugin — + // leaving no orphaned `hyp claude-hook` entries. + // @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [implements] — claude marker records prev_base_url + managed env/hook entries value[MARKER_KEY] = { attached_at: new Date().toISOString(), version, port, state_file: stateFile, + managed: { + env: { ANTHROPIC_BASE_URL: baseUrl }, + hooks: managedHookEntries(command), + }, + ...(prevBaseUrl !== undefined ? { prev_base_url: prevBaseUrl } : {}), } await writeAtomic(settingsPath, value, mtimeMs) /** @type {ClaudeAttachResult} */ const result = { changed: true } - if (prevValue !== undefined) result.prevValue = prevValue + if (prevBaseUrl !== undefined) result.prevValue = prevBaseUrl return result } @@ -303,6 +331,23 @@ function managedHookEvents() { return [...new Set(MANAGED_HOOK_SPECS.map((spec) => spec.event))] } +/** + * The managed session-context hook entries this attach installs, + * recorded into the marker's undo record so the core undo can strip + * exactly what `installSessionContextHooks` added without re-deriving + * them from the (possibly unloaded) plugin. + * + * @param {string} command + * @returns {{ event: string, matcher?: string, command: string }[]} + */ +function managedHookEntries(command) { + return MANAGED_HOOK_SPECS.map((spec) => ({ + event: spec.event, + ...(spec.matcher ? { matcher: spec.matcher } : {}), + command, + })) +} + /** @param {unknown} group */ function removeManagedHandlers(group) { if (!isPlainObject(group)) return group diff --git a/test/plugins/claude-settings-attach.test.js b/test/plugins/claude-settings-attach.test.js new file mode 100644 index 0000000..cbf6003 --- /dev/null +++ b/test/plugins/claude-settings-attach.test.js @@ -0,0 +1,152 @@ +// @ts-check + +import assert from 'node:assert/strict' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import test from 'node:test' + +import { attach } from '../../hypaware-core/plugins-workspace/claude/src/settings.js' + +/** + * T1 (LLP 0045/0046): the Claude `_hypaware` marker is a self-describing + * undo record. `attach()` records everything the format-aware core undo + * (task 4) needs to reverse the attach from disk alone — `prev_base_url` + * (the restore target) plus the managed `env.ANTHROPIC_BASE_URL` and the + * managed session-context hook entries — so reverse never depends on the + * plugin being loaded. These tests assert the marker contents directly. + */ + +/** @returns {Promise<{ dir: string, settingsPath: string }>} */ +async function stage() { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'hyp-claude-settings-')) + return { dir, settingsPath: path.join(dir, 'settings.json') } +} + +/** + * @param {string} settingsPath + * @returns {Promise>} + */ +async function readMarker(settingsPath) { + const parsed = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + return parsed._hypaware +} + +const ATTACH = { port: 4123, version: '0.2.0', stateFile: '/abs/session-context.jsonl' } + +test('attach records the managed env + hook entries into the marker undo record', async () => { + const { dir, settingsPath } = await stage() + try { + await fs.writeFile(settingsPath, JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-x' } }, null, 2)) + + const result = await attach({ ...ATTACH, settingsPath }) + assert.equal(result.changed, true) + + const marker = await readMarker(settingsPath) + // Managed env value is the gateway URL we wrote — the core undo + // matches the live value against it before removing. + assert.deepEqual(marker.managed.env, { ANTHROPIC_BASE_URL: 'http://127.0.0.1:4123' }) + + // Every managed hook spec is recorded with its command, and the + // PostToolUse entry carries its matcher so the undo strips exactly + // what was installed. + const events = marker.managed.hooks.map((/** @type {any} */ h) => h.event).sort() + assert.deepEqual(events, ['CwdChanged', 'PostToolUse', 'SessionStart', 'UserPromptSubmit']) + for (const hook of marker.managed.hooks) { + assert.match(hook.command, /claude-hook session-context --state-file /) + } + const postToolUse = marker.managed.hooks.find((/** @type {any} */ h) => h.event === 'PostToolUse') + assert.equal(postToolUse.matcher, 'Bash') + const sessionStart = marker.managed.hooks.find((/** @type {any} */ h) => h.event === 'SessionStart') + assert.equal(sessionStart.matcher, undefined) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) + +test('attach backs up a pre-existing foreign ANTHROPIC_BASE_URL as prev_base_url', async () => { + const { dir, settingsPath } = await stage() + try { + await fs.writeFile( + settingsPath, + JSON.stringify({ env: { ANTHROPIC_BASE_URL: 'https://foreign.example/api' } }, null, 2) + ) + + const result = await attach({ ...ATTACH, settingsPath }) + assert.equal(result.changed && result.prevValue, 'https://foreign.example/api') + + const marker = await readMarker(settingsPath) + assert.equal(marker.prev_base_url, 'https://foreign.example/api') + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) + +test('attach omits prev_base_url when there was no pre-existing base URL', async () => { + const { dir, settingsPath } = await stage() + try { + const result = await attach({ ...ATTACH, settingsPath }) + assert.equal(result.changed, true) + assert.equal('prevValue' in result, false) + + const marker = await readMarker(settingsPath) + assert.equal('prev_base_url' in marker, false) + // The managed undo record is still present so the core undo can + // remove (not restore) the gateway URL. + assert.deepEqual(marker.managed.env, { ANTHROPIC_BASE_URL: 'http://127.0.0.1:4123' }) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) + +test('idempotent re-attach keeps the original prev_base_url, not the gateway URL', async () => { + const { dir, settingsPath } = await stage() + try { + await fs.writeFile( + settingsPath, + JSON.stringify({ env: { ANTHROPIC_BASE_URL: 'https://foreign.example/api' } }, null, 2) + ) + + await attach({ ...ATTACH, settingsPath }) + const second = await attach({ ...ATTACH, settingsPath }) + + // The second attach observes our gateway URL live, but must report + // and record the *original* foreign URL — not the gateway URL. + assert.equal(second.changed && second.prevValue, 'https://foreign.example/api') + const marker = await readMarker(settingsPath) + assert.equal(marker.prev_base_url, 'https://foreign.example/api') + assert.equal(marker.managed.env.ANTHROPIC_BASE_URL, 'http://127.0.0.1:4123') + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) + +test('idempotent re-attach does not invent a prev_base_url when none existed', async () => { + const { dir, settingsPath } = await stage() + try { + await attach({ ...ATTACH, settingsPath }) + const second = await attach({ ...ATTACH, settingsPath }) + + assert.equal('prevValue' in second, false) + const marker = await readMarker(settingsPath) + assert.equal('prev_base_url' in marker, false) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) + +test('the marker undo record is stable across re-attach (modulo attached_at)', async () => { + const { dir, settingsPath } = await stage() + try { + await attach({ ...ATTACH, settingsPath }) + const first = await readMarker(settingsPath) + await attach({ ...ATTACH, settingsPath }) + const second = await readMarker(settingsPath) + + delete first.attached_at + delete second.attached_at + assert.deepEqual(second, first) + } finally { + await fs.rm(dir, { recursive: true, force: true }) + } +}) diff --git a/test/plugins/codex-toml-config.test.js b/test/plugins/codex-toml-config.test.js index 23193fb..d63aac3 100644 --- a/test/plugins/codex-toml-config.test.js +++ b/test/plugins/codex-toml-config.test.js @@ -42,6 +42,35 @@ test('prepareAttach is idempotent for an already managed config', () => { assert.equal(isManagedAttached(twice.content), true) }) +// T1 (LLP 0045/0046): the codex `# BEGIN/END hypaware` marked block is a +// self-describing undo record — the block is self-delimiting and records +// the prior `model_provider` as `# previous_model_provider`, so the +// format-aware core undo (task 4) can strip the block and restore the +// pointer without loading the codex plugin. +test('prepareAttach records the prior model_provider in the marked block undo record', () => { + const result = prepareAttach('model_provider = "openai"\n', 4388, '0.2.0') + + // Self-delimiting marked blocks (what the core undo strips by format). + assert.match(result.content, /# BEGIN hypaware codex model_provider/) + assert.match(result.content, /# END hypaware codex model_provider/) + assert.match(result.content, /# BEGIN hypaware codex provider/) + assert.match(result.content, /# END hypaware codex provider/) + // The prior pointer (the restore target) lives inside the root block. + assert.match(result.content, /# previous_model_provider = "openai"/) + assert.equal(result.prevValue, 'openai') +}) + +test('re-attach keeps the original previous_model_provider, not the managed one', () => { + const once = prepareAttach('model_provider = "openai"\n', 4388, '0.2.0') + // A second attach observes our managed `model_provider = "hypaware"` + // live, but must keep the marked block's recorded original. + const twice = prepareAttach(once.content, 4388, '0.2.0') + + assert.equal(twice.prevValue, 'openai') + assert.equal(twice.content.match(/# previous_model_provider = "openai"/g)?.length, 1) + assert.equal(twice.content.match(/# previous_model_provider = "hypaware"/g) ?? null, null) +}) + test('prepareDetach removes managed Codex blocks and restores previous provider', () => { const attached = prepareAttach('model_provider = "openai"\n', 4388, '0.2.0') const detached = prepareDetach(attached.content) From 4c282621a278cdf735f463f1d3dbb5c1bb97b477 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:30:29 -0700 Subject: [PATCH 06/15] The single core undo (= detach): disk-driven client_detach_disk.js (LLP 0045/0046 T4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `src/core/config/client_detach_disk.js` — the one detach implementation. It reverses a client's attach from disk state alone: the descriptor's `attachProbe` locates the settings file and the settings-file marker is the self-describing undo record `attach()` wrote (LLP 0045 Part 3, the contract task T1 landed). Format-aware but plugin-agnostic, reusing the same `resolveClientSettingsPath` + `attachProbe` format dispatch (`json` marker-key / `toml` managed-block) the read-side `probeClientAttached` uses, and importing no plugin code (which would not survive the plugin being unloaded at reverse time): - json (claude `_hypaware`): restore-or-remove each managed `env` key from the recorded `prev_base_url`, strip the recorded managed hook entries (matched by event/matcher/exact command so no orphaned `hyp claude-hook` survives), and delete the marker. A pre-existing foreign base URL round-trips byte-for-byte; the no-pre-existing-URL fixture round-trips to empty; an externally-overridden value is left in place with a warning. - toml (codex `# BEGIN/END hypaware …`): strip the self-delimiting managed blocks and restore the prior `model_provider` recorded as `# previous_model_provider`. The managed-block convention is now a core-understood format contract, subsuming the codex adapter's old marked-block strip + model_provider restore. Atomic, mtime-gated writes preserve the adapters' concurrent-edit guarantee. Unit-tested on fixture settings files with no plugin loaded (incl. hand-written markers), proving reverse never depends on `ctx.clients`; cross-checked against `probeClientAttachFromDescriptor` for both formats. No second caller is wired yet (T5 reroutes `hyp detach`, T6 the reconciler `reverse()`), so this lands inert and green. Co-Authored-By: Claude Opus 4.8 (1M context) Task-Id: T4 --- src/core/config/client_detach_disk.js | 662 ++++++++++++++++++++++++++ test/core/client-detach-disk.test.js | 368 ++++++++++++++ 2 files changed, 1030 insertions(+) create mode 100644 src/core/config/client_detach_disk.js create mode 100644 test/core/client-detach-disk.test.js diff --git a/src/core/config/client_detach_disk.js b/src/core/config/client_detach_disk.js new file mode 100644 index 0000000..10cab06 --- /dev/null +++ b/src/core/config/client_detach_disk.js @@ -0,0 +1,662 @@ +// @ts-check + +import crypto from 'node:crypto' +import fsp from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' + +import { resolveClientSettingsPath } from '../daemon/client_settings_path.js' + +/** + * @import { FileHandle } from 'node:fs/promises' + * @import { ClientDescriptor } from '../plugin_catalog.js' + */ + +/** + * The single core undo — the disk-driven, plugin-agnostic reverse of a + * client's attach. It is the *one* detach implementation: both the reconciler's + * `reverse()` (a fleet-config drop, fired only after the staged restart has + * already unloaded the adapter) and the manual `hyp detach` command route + * through it, so there is no second implementation to drift from. + * + * Reverse runs from **disk state alone** — the descriptor's `attachProbe` + * locates the settings file, and the client's own settings-file marker is a + * **self-describing undo record** that `attach()` wrote (LLP 0045 §Part 3). The + * routine is **format-aware but plugin-agnostic**: it understands `json` + * (marker-key) and `toml` (managed-block) — the same dispatch + * `probeClientAttached` uses on the *read* side — and how to replay an undo + * record, never "Claude" vs "Codex". It imports no plugin code (which would not + * survive the plugin being unloaded), subsuming what the adapters' old + * `detach()` did — including the Codex `# BEGIN/END hypaware …` marked-block + * strip and prior-`model_provider` restore. The managed-block convention is + * therefore a **core-understood format contract**, not a codex-private detail. + * + * @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [implements] — one core/disk-driven undo, format-aware (json marker-key / toml managed-block), plugin-agnostic, reusing resolveClientSettingsPath + the probeClientAttached format dispatch + * @ref LLP 0044#conflict--back-up--override-restore-on-leave [constrained-by] — the marker is the backup; reverse restores it (or removes the managed value) on leave + */ + +const TOML_MANAGED_BEGIN = '# BEGIN hypaware' +const TOML_MANAGED_END = '# END hypaware' +const TOML_PREVIOUS_KEY = 'previous_model_provider' +const TOML_ROOT_RESTORE_KEY = 'model_provider' +const TOML_MANAGED_BASE_URL_KEY = 'base_url' + +const TOML_BASIC_MULTILINE_DELIMITER = '"""' +const TOML_LITERAL_MULTILINE_DELIMITER = '\'\'\'' +const TOML_KEY_PART = String.raw`(?:"(?:\\.|[^"\\])*"|'[^']*'|[A-Za-z0-9_-]+)` +const TOML_DOTTED_KEY = String.raw`${TOML_KEY_PART}(?:\s*\.\s*${TOML_KEY_PART})*` +const TOML_TABLE_HEADER_RE = new RegExp(String.raw`^\s*\[\s*${TOML_DOTTED_KEY}\s*\]\s*(?:#.*)?$`) +const TOML_TABLE_ARRAY_HEADER_RE = new RegExp(String.raw`^\s*\[\[\s*${TOML_DOTTED_KEY}\s*\]\]\s*(?:#.*)?$`) +const TOML_ROOT_MODEL_PROVIDER_RE = new RegExp( + String.raw`^\s*(?:${TOML_ROOT_RESTORE_KEY}|"${TOML_ROOT_RESTORE_KEY}"|'${TOML_ROOT_RESTORE_KEY}')\s*=` +) + +export class ClientDetachError extends Error { + /** + * @param {string} message + * @param {{ code?: string, cause?: unknown }} [opts] + */ + constructor(message, opts = {}) { + super(message) + this.name = 'ClientDetachError' + /** @type {string | undefined} */ + this.code = opts.code + if (opts.cause !== undefined) { + /** @type {unknown} */ + this.cause = opts.cause + } + } +} + +/** + * @typedef {object} DetachFromDiskResult + * @property {boolean} changed True when the settings file was rewritten. + * @property {string} [settingsPath] The resolved settings path (when one exists). + * @property {string} [removed] The managed value deleted (e.g. the gateway base URL) when there was no prior to restore. + * @property {string} [restoredValue] The prior value restored from the undo record. + * @property {string} [warning] Set when the managed value was overridden externally and left in place. + */ + +/** + * Reverse a client's attach from disk, driven by the descriptor's + * `attachProbe` and the settings-file marker. No-op (`{ changed: false }`) when + * the descriptor has no probe, the file is absent, or it carries no marker. + * + * @param {{ + * descriptor: ClientDescriptor, + * homeDir?: string, + * env?: NodeJS.ProcessEnv, + * fs?: typeof fsp, + * }} args + * @returns {Promise} + */ +export async function detachClientFromDisk({ descriptor, homeDir = os.homedir(), env, fs = fsp }) { + const probe = descriptor.attachProbe + if (!probe) return { changed: false } + + const settingsPath = resolveClientSettingsPath(descriptor.name, probe.settings_file, env, homeDir) + + if (probe.format === 'json' && probe.marker_key) { + return await detachJsonMarker({ settingsPath, markerKey: probe.marker_key, fs }) + } + if (probe.format === 'toml') { + return await detachTomlManagedBlock({ settingsPath, fs }) + } + // Unknown/incomplete probe: nothing this core routine knows how to reverse. + return { changed: false, settingsPath } +} + +/* ------------------------------- JSON format ------------------------------ */ + +/** + * Reverse a `json` marker-key attach (e.g. Claude's `_hypaware`). Replays the + * self-describing undo record: restore-or-remove each managed `env` key, + * strip the recorded managed hook entries (leaving no orphaned `hyp …` hooks), + * and delete the marker. + * + * @param {{ settingsPath: string, markerKey: string, fs: typeof fsp }} args + * @returns {Promise} + */ +async function detachJsonMarker({ settingsPath, markerKey, fs }) { + const read = await readJson(settingsPath, fs) + if (!read.existed) return { changed: false, settingsPath } + + const value = read.value + const marker = value[markerKey] + if (!isPlainObject(marker)) return { changed: false, settingsPath } + + const managed = isPlainObject(marker.managed) ? marker.managed : {} + const managedEnv = isPlainObject(managed.env) ? managed.env : {} + const hookEntries = Array.isArray(managed.hooks) ? managed.hooks : [] + const prevBaseUrl = typeof marker.prev_base_url === 'string' ? marker.prev_base_url : undefined + + delete value[markerKey] + stripManagedHooks(value, hookEntries) + + /** @type {string | undefined} */ + let removed + /** @type {string | undefined} */ + let restoredValue + /** @type {string | undefined} */ + let warning + + if (isPlainObject(value.env)) { + const envObj = /** @type {Record} */ (value.env) + for (const [key, ourVal] of Object.entries(managedEnv)) { + const current = envObj[key] + if (current === ourVal) { + // The value we wrote is still live: restore the recorded prior, or + // remove the key when the attach had no pre-existing value to back up. + if (prevBaseUrl !== undefined) { + envObj[key] = prevBaseUrl + restoredValue = prevBaseUrl + } else { + removed = typeof current === 'string' ? current : String(current) + delete envObj[key] + } + } else if (typeof current === 'string') { + // Overridden externally after we attached — never clobber a user edit. + warning = `${key} was overridden externally; leaving in place` + } + } + if (Object.keys(envObj).length === 0) delete value.env + } + + await writeJsonAtomic(settingsPath, value, read.mtimeMs, fs) + + /** @type {DetachFromDiskResult} */ + const result = { changed: true, settingsPath } + if (removed !== undefined) result.removed = removed + if (restoredValue !== undefined) result.restoredValue = restoredValue + if (warning !== undefined) result.warning = warning + return result +} + +/** + * Strip the managed hook entries the marker recorded — matching each by its + * `event` / `matcher` / exact `command`, so only the handlers this attach + * installed are removed and no orphaned `hyp …` hooks survive. Empty groups + * and empty event arrays are pruned; an emptied `hooks` root is deleted. + * + * @param {Record} value + * @param {unknown[]} hookEntries + */ +function stripManagedHooks(value, hookEntries) { + const hooksRoot = value.hooks + if (!isPlainObject(hooksRoot)) return + + for (const entry of hookEntries) { + if (!isPlainObject(entry)) continue + const event = typeof entry.event === 'string' ? entry.event : undefined + const command = typeof entry.command === 'string' ? entry.command : undefined + if (event === undefined || command === undefined) continue + const matcher = typeof entry.matcher === 'string' ? entry.matcher : undefined + + const groups = hooksRoot[event] + if (!Array.isArray(groups)) continue + + /** @type {unknown[]} */ + const nextGroups = [] + for (const group of groups) { + if (!isPlainObject(group) || !groupMatcherEquals(group, matcher) || !Array.isArray(group.hooks)) { + nextGroups.push(group) + continue + } + const handlers = group.hooks + const keptHandlers = handlers.filter((h) => !isManagedHandler(h, command)) + if (keptHandlers.length === handlers.length) { + nextGroups.push(group) // nothing matched — leave the group untouched + } else if (keptHandlers.length > 0) { + nextGroups.push({ ...group, hooks: keptHandlers }) + } + // else: the group held only the managed handler — drop it entirely. + } + + if (nextGroups.length > 0) { + hooksRoot[event] = nextGroups + } else { + delete hooksRoot[event] + } + } + + if (Object.keys(hooksRoot).length === 0) delete value.hooks +} + +/** + * @param {Record} group + * @param {string | undefined} matcher + */ +function groupMatcherEquals(group, matcher) { + const groupMatcher = typeof group.matcher === 'string' ? group.matcher : undefined + return groupMatcher === matcher +} + +/** + * @param {unknown} handler + * @param {string} command + */ +function isManagedHandler(handler, command) { + return isPlainObject(handler) && handler.type === 'command' && handler.command === command +} + +/* ------------------------------- TOML format ------------------------------ */ + +/** + * Reverse a `toml` managed-block attach (e.g. Codex's `# BEGIN/END hypaware …` + * blocks). The blocks are self-delimiting and record the prior `model_provider` + * as `# previous_model_provider`, so core strips the blocks and restores the + * recorded root pointer — without importing the codex plugin. + * + * @param {{ settingsPath: string, fs: typeof fsp }} args + * @returns {Promise} + */ +async function detachTomlManagedBlock({ settingsPath, fs }) { + const read = await readText(settingsPath, fs) + if (!read.existed) return { changed: false, settingsPath } + + const lines = splitLines(read.content) + const blockValues = readManagedBlockValues(lines) + if (!blockValues.found) return { changed: false, settingsPath } + + let next = removeManagedBlocks(lines) + + /** @type {string | undefined} */ + let restoredValue + /** @type {string | undefined} */ + let warning + if (blockValues.previous !== undefined) { + const current = readRootModelProvider(next) + if (current === undefined) { + next = insertRootLines(next, [`${TOML_ROOT_RESTORE_KEY} = ${tomlString(blockValues.previous)}`]) + restoredValue = blockValues.previous + } else if (current !== blockValues.previous) { + warning = `${TOML_ROOT_RESTORE_KEY} was changed externally; leaving ${current} in place` + } + } + + await writeTextAtomic(settingsPath, formatLines(next), read.mtimeMs, fs) + + /** @type {DetachFromDiskResult} */ + const result = { changed: true, settingsPath } + if (blockValues.removed !== undefined) result.removed = blockValues.removed + if (restoredValue !== undefined) result.restoredValue = restoredValue + if (warning !== undefined) result.warning = warning + return result +} + +/** + * Single pass over the managed blocks: detect their presence and read the prior + * `model_provider` (the restore target, recorded as a `# previous_model_provider` + * comment) and the managed `base_url` (reported as `removed`). + * + * @param {string[]} lines + * @returns {{ found: boolean, previous?: string, removed?: string }} + */ +function readManagedBlockValues(lines) { + const prevRe = new RegExp(String.raw`^#\s*${TOML_PREVIOUS_KEY}\s*=\s*(.+)$`) + const baseRe = new RegExp(String.raw`^\s*${TOML_MANAGED_BASE_URL_KEY}\s*=\s*(.+)$`) + let inside = false + let found = false + /** @type {string | undefined} */ + let previous + /** @type {string | undefined} */ + let removed + for (const line of lines) { + const trimmed = line.trim() + if (!inside) { + if (trimmed.startsWith(TOML_MANAGED_BEGIN)) { + inside = true + found = true + } + continue + } + if (trimmed.startsWith(TOML_MANAGED_END)) { + inside = false + continue + } + if (previous === undefined) { + const m = line.match(prevRe) + if (m) previous = parseTomlString(m[1]) + } + if (removed === undefined) { + const m = line.match(baseRe) + if (m) removed = parseTomlString(m[1]) + } + } + /** @type {{ found: boolean, previous?: string, removed?: string }} */ + const result = { found } + if (previous !== undefined) result.previous = previous + if (removed !== undefined) result.removed = removed + return result +} + +/** + * Strip every `# BEGIN hypaware …` … `# END hypaware …` block (inclusive). The + * convention is self-delimiting, so this removes exactly what attach inserted. + * + * @param {string[]} lines + * @returns {string[]} + */ +function removeManagedBlocks(lines) { + /** @type {string[]} */ + const next = [] + for (let i = 0; i < lines.length; i++) { + if (!lines[i].trim().startsWith(TOML_MANAGED_BEGIN)) { + next.push(lines[i]) + continue + } + let foundEnd = false + for (i++; i < lines.length; i++) { + if (lines[i].trim().startsWith(TOML_MANAGED_END)) { + foundEnd = true + break + } + } + if (!foundEnd) { + throw new ClientDetachError('unterminated hypaware-managed config block', { + code: 'MALFORMED_MARKER', + }) + } + } + return next +} + +/** + * Read the root `model_provider` (before the first table header), honoring + * multiline strings so a `"""…"""` value can't be misread as an assignment. + * + * @param {string[]} lines + * @returns {string | undefined} + */ +function readRootModelProvider(lines) { + const firstTable = findFirstTableIndex(lines) + /** @type {string | undefined} */ + let multilineDelimiter + for (let i = 0; i < firstTable; i++) { + if (multilineDelimiter !== undefined) { + multilineDelimiter = closeMultilineString(lines[i], multilineDelimiter) + continue + } + if (TOML_ROOT_MODEL_PROVIDER_RE.test(lines[i])) return parseAssignmentString(lines[i]) + multilineDelimiter = openMultilineString(lines[i]) + } + return undefined +} + +/** + * Insert lines at the root (before the first table header / trailing blanks). + * + * @param {string[]} lines + * @param {string[]} insert + * @returns {string[]} + */ +function insertRootLines(lines, insert) { + const next = lines.slice() + let index = findFirstTableIndex(next) + if (index === next.length) { + while (index > 0 && next[index - 1] === '') index-- + } + next.splice(index, 0, ...insert) + return next +} + +/** @param {string[]} lines */ +function findFirstTableIndex(lines) { + /** @type {string | undefined} */ + let multilineDelimiter + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + if (multilineDelimiter !== undefined) { + multilineDelimiter = closeMultilineString(line, multilineDelimiter) + continue + } + if (isTableHeader(line)) return i + multilineDelimiter = openMultilineString(line) + } + return lines.length +} + +/** @param {string} line */ +function isTableHeader(line) { + return TOML_TABLE_HEADER_RE.test(line) || TOML_TABLE_ARRAY_HEADER_RE.test(line) +} + +/** @param {string} line */ +function openMultilineString(line) { + const value = assignmentValue(line) ?? line.trimStart() + if (value.startsWith(TOML_BASIC_MULTILINE_DELIMITER)) { + return hasClosingMultilineString(value.slice(3), TOML_BASIC_MULTILINE_DELIMITER) + ? undefined + : TOML_BASIC_MULTILINE_DELIMITER + } + if (value.startsWith(TOML_LITERAL_MULTILINE_DELIMITER)) { + return hasClosingMultilineString(value.slice(3), TOML_LITERAL_MULTILINE_DELIMITER) + ? undefined + : TOML_LITERAL_MULTILINE_DELIMITER + } + return undefined +} + +/** + * @param {string} line + * @param {string} delimiter + */ +function closeMultilineString(line, delimiter) { + return hasClosingMultilineString(line, delimiter) ? undefined : delimiter +} + +/** @param {string} line */ +function assignmentValue(line) { + if (/^\s*#/.test(line)) return undefined + const index = line.indexOf('=') + return index === -1 ? undefined : line.slice(index + 1).trimStart() +} + +/** + * @param {string} value + * @param {string} delimiter + */ +function hasClosingMultilineString(value, delimiter) { + if (delimiter === TOML_LITERAL_MULTILINE_DELIMITER) return value.includes(delimiter) + for (let index = value.indexOf(delimiter); index !== -1; index = value.indexOf(delimiter, index + 1)) { + if (!isEscaped(value, index)) return true + } + return false +} + +/** + * @param {string} value + * @param {number} index + */ +function isEscaped(value, index) { + let backslashes = 0 + for (let i = index - 1; i >= 0 && value[i] === '\\'; i--) backslashes++ + return backslashes % 2 === 1 +} + +/** @param {string} line */ +function parseAssignmentString(line) { + const index = line.indexOf('=') + if (index === -1) return undefined + return parseTomlString(line.slice(index + 1)) +} + +/** @param {string} value */ +function parseTomlString(value) { + const trimmed = value.trim() + if (trimmed.startsWith('"')) { + const match = trimmed.match(/^"(?:\\.|[^"\\])*"/) + if (!match) return undefined + try { return JSON.parse(match[0]) } catch { return undefined } + } + if (trimmed.startsWith('\'')) { + const match = trimmed.match(/^'([^']*)'/) + return match ? match[1] : undefined + } + return undefined +} + +/** @param {string} value */ +function tomlString(value) { + return JSON.stringify(value) +} + +/** + * @param {string} content + * @returns {string[]} + */ +function splitLines(content) { + const normalized = content.replace(/\r\n/g, '\n').replace(/\r/g, '\n') + if (normalized === '') return [] + const lines = normalized.split('\n') + if (lines[lines.length - 1] === '') lines.pop() + return lines +} + +/** + * @param {string[]} lines + * @returns {string} + */ +function formatLines(lines) { + let start = 0 + let end = lines.length + while (start < end && lines[start] === '') start++ + while (end > start && lines[end - 1] === '') end-- + const out = lines.slice(start, end) + return out.length === 0 ? '' : `${out.join('\n')}\n` +} + +/* --------------------------------- I/O ------------------------------------ */ + +/** + * @param {string} settingsPath + * @param {typeof fsp} fs + * @returns {Promise<{ value: Record, existed: boolean, mtimeMs: number | undefined }>} + */ +async function readJson(settingsPath, fs) { + const read = await readText(settingsPath, fs) + if (!read.existed) return { value: {}, existed: false, mtimeMs: undefined } + + /** @type {unknown} */ + let parsed + try { + parsed = JSON.parse(read.content) + } catch (err) { + throw new ClientDetachError(`malformed JSON in ${settingsPath}: ${errMsg(err)}`, { + code: 'MALFORMED_JSON', + cause: err, + }) + } + if (!isPlainObject(parsed)) { + throw new ClientDetachError(`${settingsPath} must contain a JSON object at the root`, { + code: 'NOT_AN_OBJECT', + }) + } + return { value: parsed, existed: true, mtimeMs: read.mtimeMs } +} + +/** + * @param {string} settingsPath + * @param {typeof fsp} fs + * @returns {Promise<{ content: string, existed: boolean, mtimeMs: number | undefined }>} + */ +async function readText(settingsPath, fs) { + /** @type {string} */ + let raw + try { + raw = await fs.readFile(settingsPath, 'utf8') + } catch (err) { + if (errCode(err) === 'ENOENT') return { content: '', existed: false, mtimeMs: undefined } + throw new ClientDetachError(`failed to read ${settingsPath}: ${errMsg(err)}`, { cause: err }) + } + let stat + try { + stat = await fs.stat(settingsPath) + } catch (err) { + throw new ClientDetachError(`failed to stat ${settingsPath}: ${errMsg(err)}`, { cause: err }) + } + return { content: raw, existed: true, mtimeMs: stat.mtimeMs } +} + +/** + * @param {string} settingsPath + * @param {unknown} value + * @param {number | undefined} expectedMtimeMs + * @param {typeof fsp} fs + */ +async function writeJsonAtomic(settingsPath, value, expectedMtimeMs, fs) { + await writeTextAtomic(settingsPath, JSON.stringify(value, null, 2) + '\n', expectedMtimeMs, fs) +} + +/** + * Atomic temp-file + rename write, gated on the file's mtime so a concurrent + * edit between read and write is detected (CONCURRENT_EDIT) rather than + * silently clobbered — the same guarantee the adapters' writers gave. + * + * @param {string} filePath + * @param {string} body + * @param {number | undefined} expectedMtimeMs + * @param {typeof fsp} fs + */ +async function writeTextAtomic(filePath, body, expectedMtimeMs, fs) { + if (expectedMtimeMs !== undefined) { + let current + try { + current = await fs.stat(filePath) + } catch (err) { + if (errCode(err) === 'ENOENT') { + throw new ClientDetachError(`${filePath} disappeared between read and write; retry`, { + code: 'CONCURRENT_EDIT', + cause: err, + }) + } + throw err + } + if (current.mtimeMs !== expectedMtimeMs) { + throw new ClientDetachError(`${filePath} changed on disk between read and write; retry`, { + code: 'CONCURRENT_EDIT', + }) + } + } + + await fs.mkdir(path.dirname(filePath), { recursive: true }) + + const tmpPath = `${filePath}.${process.pid}.${crypto.randomBytes(6).toString('hex')}.tmp` + /** @type {FileHandle | undefined} */ + let handle + try { + handle = await fs.open(tmpPath, 'w', 0o600) + await handle.writeFile(body, 'utf8') + await handle.sync() + } finally { + if (handle) await handle.close() + } + try { + await fs.rename(tmpPath, filePath) + } catch (err) { + await fs.rm(tmpPath, { force: true }) + throw err + } +} + +/* ------------------------------- Utilities -------------------------------- */ + +/** + * @param {unknown} value + * @returns {value is Record} + */ +function isPlainObject(value) { + return value !== null && typeof value === 'object' && !Array.isArray(value) +} + +/** @param {unknown} err */ +function errCode(err) { + if (!err || typeof err !== 'object' || !('code' in err)) return undefined + const code = Reflect.get(err, 'code') + return typeof code === 'string' ? code : undefined +} + +/** @param {unknown} err */ +function errMsg(err) { + return err instanceof Error ? err.message : String(err) +} diff --git a/test/core/client-detach-disk.test.js b/test/core/client-detach-disk.test.js new file mode 100644 index 0000000..3a2f689 --- /dev/null +++ b/test/core/client-detach-disk.test.js @@ -0,0 +1,368 @@ +// @ts-check + +import assert from 'node:assert/strict' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import test from 'node:test' + +import { detachClientFromDisk } from '../../src/core/config/client_detach_disk.js' +import { probeClientAttachFromDescriptor } from '../../src/core/daemon/status.js' +// Adapter helpers are used only to *build* realistic fixtures. The core undo +// under test imports no plugin code — these prove the round-trip against what +// `attach()` actually wrote (LLP 0045 §Part 3, task T4). +import { attach as claudeAttach } from '../../hypaware-core/plugins-workspace/claude/src/settings.js' +import { prepareAttach as codexPrepareAttach } from '../../hypaware-core/plugins-workspace/codex/src/toml-config.js' + +/** + * T4 (LLP 0045/0046): the single core undo (= detach). `client_detach_disk.js` + * reverses a client's attach from disk alone — the descriptor's `attachProbe` + * plus the settings-file marker, format-aware (json marker-key / toml + * managed-block) but plugin-agnostic. These tests run the undo with **no plugin + * loaded** at reverse time, proving it never depends on `ctx.clients`. + */ + +/** @import { ClientDescriptor } from '../../src/core/plugin_catalog.js' */ + +/** @type {ClientDescriptor} */ +const CLAUDE_DESCRIPTOR = { + plugin: /** @type {any} */ ('@hypaware/claude'), + name: 'claude', + skillDir: 'skills/claude', + attachProbe: { format: 'json', settings_file: '.claude/settings.json', marker_key: '_hypaware' }, +} + +/** @type {ClientDescriptor} */ +const CODEX_DESCRIPTOR = { + plugin: /** @type {any} */ ('@hypaware/codex'), + name: 'codex', + skillDir: 'skills/codex', + attachProbe: { format: 'toml', settings_file: '.codex/config.toml', marker_header: '[model_providers.hypaware]' }, +} + +const ATTACH = { port: 4123, version: '0.2.0', stateFile: '/abs/session-context.jsonl' } + +/** @returns {Promise} */ +async function stageHome() { + return await fs.mkdtemp(path.join(os.tmpdir(), 'hyp-detach-disk-')) +} + +/** + * @param {string} home + * @param {string} content + * @returns {Promise} + */ +async function writeClaudeSettings(home, content) { + const p = path.join(home, '.claude', 'settings.json') + await fs.mkdir(path.dirname(p), { recursive: true }) + await fs.writeFile(p, content) + return p +} + +/** + * @param {string} home + * @param {string} content + * @returns {Promise} + */ +async function writeCodexConfig(home, content) { + const p = path.join(home, '.codex', 'config.toml') + await fs.mkdir(path.dirname(p), { recursive: true }) + await fs.writeFile(p, content) + return p +} + +/* -------------------------------- claude (json) -------------------------------- */ + +test('claude undo restores a pre-existing foreign base URL byte-for-byte', async () => { + const home = await stageHome() + try { + const original = { env: { ANTHROPIC_API_KEY: 'sk-x', ANTHROPIC_BASE_URL: 'https://foreign.example/api' } } + const originalText = JSON.stringify(original, null, 2) + '\n' + const settingsPath = await writeClaudeSettings(home, originalText) + + // Build the fixture with the real adapter (test setup only). + await claudeAttach({ ...ATTACH, settingsPath }) + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.restoredValue, 'https://foreign.example/api') + assert.equal(result.settingsPath, settingsPath) + + assert.equal(await fs.readFile(settingsPath, 'utf8'), originalText) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo of a no-pre-existing-URL attach round-trips to empty', async () => { + const home = await stageHome() + try { + const originalText = JSON.stringify({}, null, 2) + '\n' + const settingsPath = await writeClaudeSettings(home, originalText) + await claudeAttach({ ...ATTACH, settingsPath }) + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.removed, 'http://127.0.0.1:4123') + assert.equal('restoredValue' in result, false) + + assert.equal(await fs.readFile(settingsPath, 'utf8'), originalText) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo strips marker + managed keys/hooks from a hand-written fixture (no plugin loaded)', async () => { + const home = await stageHome() + try { + const command = "hyp claude-hook session-context --state-file '/abs/session-context.jsonl'" + const fixture = { + env: { ANTHROPIC_API_KEY: 'sk-x', ANTHROPIC_BASE_URL: 'http://127.0.0.1:4123' }, + hooks: { + SessionStart: [{ hooks: [{ type: 'command', command }] }], + CwdChanged: [{ hooks: [{ type: 'command', command }] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command }] }], + PostToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command }] }], + }, + _hypaware: { + attached_at: '2026-06-26T00:00:00.000Z', + version: '0.2.0', + port: 4123, + state_file: '/abs/session-context.jsonl', + managed: { + env: { ANTHROPIC_BASE_URL: 'http://127.0.0.1:4123' }, + hooks: [ + { event: 'SessionStart', command }, + { event: 'CwdChanged', command }, + { event: 'UserPromptSubmit', command }, + { event: 'PostToolUse', matcher: 'Bash', command }, + ], + }, + prev_base_url: 'https://foreign.example/api', + }, + } + const settingsPath = await writeClaudeSettings(home, JSON.stringify(fixture, null, 2) + '\n') + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.restoredValue, 'https://foreign.example/api') + + const raw = await fs.readFile(settingsPath, 'utf8') + const parsed = JSON.parse(raw) + assert.equal('_hypaware' in parsed, false) + assert.equal('hooks' in parsed, false) // every managed hook group pruned + assert.equal(parsed.env.ANTHROPIC_BASE_URL, 'https://foreign.example/api') + assert.equal(parsed.env.ANTHROPIC_API_KEY, 'sk-x') + assert.equal(raw.includes('claude-hook'), false) // no orphaned hyp hooks + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo leaves an externally-overridden base URL in place with a warning', async () => { + const home = await stageHome() + try { + const settingsPath = await writeClaudeSettings(home, JSON.stringify({}, null, 2) + '\n') + await claudeAttach({ ...ATTACH, settingsPath }) + + // The user re-points the base URL after we attached. + const attached = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + attached.env.ANTHROPIC_BASE_URL = 'https://someone-else.example' + await fs.writeFile(settingsPath, JSON.stringify(attached, null, 2) + '\n') + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.match(String(result.warning), /overridden externally/) + + const parsed = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + assert.equal('_hypaware' in parsed, false) // marker still removed + assert.equal(parsed.env.ANTHROPIC_BASE_URL, 'https://someone-else.example') // user value untouched + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo preserves a user-owned non-managed hook for a managed event', async () => { + const home = await stageHome() + try { + const command = "hyp claude-hook session-context --state-file '/abs/session-context.jsonl'" + const fixture = { + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'echo hello' }] }, // user's own + { hooks: [{ type: 'command', command }] }, // ours + ], + }, + _hypaware: { + version: '0.2.0', + port: 4123, + managed: { env: {}, hooks: [{ event: 'SessionStart', command }] }, + }, + } + const settingsPath = await writeClaudeSettings(home, JSON.stringify(fixture, null, 2) + '\n') + + await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + + const parsed = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + assert.equal('_hypaware' in parsed, false) + assert.deepEqual(parsed.hooks.SessionStart, [{ hooks: [{ type: 'command', command: 'echo hello' }] }]) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo is a no-op when the marker is absent', async () => { + const home = await stageHome() + try { + const text = JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-x' } }, null, 2) + '\n' + const settingsPath = await writeClaudeSettings(home, text) + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, false) + assert.equal(await fs.readFile(settingsPath, 'utf8'), text) // untouched + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo is a no-op when the settings file is absent', async () => { + const home = await stageHome() + try { + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, false) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +/* -------------------------------- codex (toml) -------------------------------- */ + +test('codex undo strips the managed blocks and restores model_provider byte-for-byte', async () => { + const home = await stageHome() + try { + const original = 'model_provider = "openai"\n' + const attached = codexPrepareAttach(original, 4388, '0.2.0') + const configPath = await writeCodexConfig(home, attached.content) + + const result = await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.restoredValue, 'openai') + assert.equal(result.removed, 'http://127.0.0.1:4388/v1') + assert.equal(result.settingsPath, configPath) + + assert.equal(await fs.readFile(configPath, 'utf8'), original) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('codex undo of a no-previous-provider attach round-trips to empty', async () => { + const home = await stageHome() + try { + const attached = codexPrepareAttach('', 4388, '0.2.0') + const configPath = await writeCodexConfig(home, attached.content) + + const result = await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal('restoredValue' in result, false) + + assert.equal(await fs.readFile(configPath, 'utf8'), '') + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('codex undo preserves unrelated config alongside the restored provider', async () => { + const home = await stageHome() + try { + const original = ['model_provider = "openai"', '', '[profiles.default]', 'model = "gpt-5"', ''].join('\n') + const attached = codexPrepareAttach(original, 4388, '0.2.0') + const configPath = await writeCodexConfig(home, attached.content) + + await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + + const raw = await fs.readFile(configPath, 'utf8') + assert.equal(raw.includes('# BEGIN hypaware'), false) + assert.equal(raw.includes('[model_providers.hypaware]'), false) + assert.match(raw, /model_provider = "openai"/) + assert.match(raw, /\[profiles\.default\]\nmodel = "gpt-5"/) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('codex undo strips a hand-written marked block (no plugin loaded)', async () => { + const home = await stageHome() + try { + const fixture = [ + '# BEGIN hypaware codex model_provider', + '# attached_at = "2026-06-26T00:00:00.000Z"', + '# version = "0.2.0"', + '# port = 4388', + '# previous_model_provider = "openai"', + 'model_provider = "hypaware"', + '# END hypaware codex model_provider', + '', + '# BEGIN hypaware codex provider', + '[model_providers.hypaware]', + 'base_url = "http://127.0.0.1:4388/v1"', + '# END hypaware codex provider', + '', + ].join('\n') + const configPath = await writeCodexConfig(home, fixture) + + const result = await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.restoredValue, 'openai') + assert.equal(result.removed, 'http://127.0.0.1:4388/v1') + + assert.equal(await fs.readFile(configPath, 'utf8'), 'model_provider = "openai"\n') + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('codex undo is a no-op when no managed block is present', async () => { + const home = await stageHome() + try { + const text = 'model_provider = "openai"\n' + const configPath = await writeCodexConfig(home, text) + + const result = await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, false) + assert.equal(await fs.readFile(configPath, 'utf8'), text) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +/* ----------------------------- shared / dispatch ----------------------------- */ + +test('undo clears exactly what probeClientAttached detects, for both formats', async () => { + const home = await stageHome() + try { + // claude + const settingsPath = await writeClaudeSettings(home, JSON.stringify({}, null, 2) + '\n') + await claudeAttach({ ...ATTACH, settingsPath }) + assert.equal((await probeClientAttachFromDescriptor({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home })).attached, true) + await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal((await probeClientAttachFromDescriptor({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home })).attached, false) + + // codex + const configPath = await writeCodexConfig(home, codexPrepareAttach('model_provider = "openai"\n', 4388, '0.2.0').content) + void configPath + assert.equal((await probeClientAttachFromDescriptor({ descriptor: CODEX_DESCRIPTOR, homeDir: home })).attached, true) + await detachClientFromDisk({ descriptor: CODEX_DESCRIPTOR, homeDir: home }) + assert.equal((await probeClientAttachFromDescriptor({ descriptor: CODEX_DESCRIPTOR, homeDir: home })).attached, false) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('undo is a no-op for a descriptor without an attachProbe', async () => { + const result = await detachClientFromDisk({ + descriptor: { plugin: /** @type {any} */ ('@x/none'), name: 'none', skillDir: 'skills/none' }, + homeDir: await stageHome(), + }) + assert.equal(result.changed, false) +}) From 365f435869c46a69a11ee4e218f9d17064669cee Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:30:47 -0700 Subject: [PATCH 07/15] =?UTF-8?q?T9:=20status=20surface=20=E2=80=94=20decl?= =?UTF-8?q?ared-attach-targets=20derivation=20(LLP=200044/0045)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildClientActionsReport (src/core/daemon/status.js) gains an attach declared-targets derivation symmetric to backfill's. It reuses the clientDescriptors-derived client-adapter plugin set and the shared readAttachPolicy (T2) so status can never disagree with the attach handler about what an `attach.on_join` block means. Interpretation, per LLP 0044 §status-surface: - enabled client adapter on a joined host, no marker -> pending - attach.on_join:false, or a non-joined host -> n/a - a `done` marker -> attached (the done rendering) Attach targets are keyed by *client* name (the handler's request key, descriptor.name) — not the owning plugin — so a `done` attach marker collapses with the declared target instead of doubling it. A failed/pending attach is its own status line and never flips `overall` to degraded (client-action state is not a diagnostic). It reads markers only; it never runs a reconcile pass. Tests: mixed done/failed/pending/n-a read cleanly; on_join:false and a non-joined explicit target both render n/a while a bare local install keeps the V1 surface; a failed attach stays healthy; a done marker collapses with its declared target (no double row). Task-Id: T9 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/daemon/status.js | 113 ++++++++++----- test/core/status-client-actions.test.js | 181 +++++++++++++++++++++++- 2 files changed, 258 insertions(+), 36 deletions(-) diff --git a/src/core/daemon/status.js b/src/core/daemon/status.js index 3fff0b0..b5fbef3 100644 --- a/src/core/daemon/status.js +++ b/src/core/daemon/status.js @@ -8,6 +8,7 @@ import process from 'node:process' import { defaultConfigPath, loadConfigFile } from '../config/schema.js' import { readConfigControlStatus, resolveCentralLayerPath } from '../config/apply.js' import { readClientActionStatus } from '../config/action_reconciler.js' +import { readAttachPolicy } from '../config/attach_policy.js' import { readBackfillPolicy } from '../config/backfill_policy.js' import { resolveLayeredConfig } from '../config/merge.js' import { devTelemetryDir, readObservabilityEnv } from '../observability/env.js' @@ -35,7 +36,7 @@ import { } from './pid.js' /** - * @import { HypAwareV2Config } from '../../../collectivus-plugin-kernel-types.d.ts' + * @import { HypAwareV2Config, PluginConfigInstance } from '../../../collectivus-plugin-kernel-types.d.ts' * @import { ClientActionStatus, ConfigControlStatus, ConfigLayerDrop, ConfigValidationError, V1Diagnostic } from '../config/types.d.ts' * @import { ClientActionReport, ClientActionsReport, ClientAttachReport, CollectStatusOptions, DaemonState, DaemonStatus, HypAwareStatusReport, ServiceState, SinkSnapshot, SourceSnapshot, StatusDiagnostic, StatusDiagnosticKind } from './types.d.ts' * @import { Dirent } from 'node:fs' @@ -489,15 +490,13 @@ export async function collectHypAwareStatus(opts = {}) { let clientActions = null try { const actionStatus = readClientActionStatus({ stateRoot }) - // Backfill-capable plugins, derived statically from the catalog's client - // descriptors (claude/codex). Status cannot see the runtime backfill - // registry without activating plugins, so the client descriptors are the - // honest static proxy for "this enabled plugin imports on join". - /** @type {Set} */ - const backfillPlugins = new Set( - [...(catalog?.clientDescriptors?.values() ?? [])].map((d) => d.plugin) - ) - clientActions = buildClientActionsReport({ status: actionStatus, config, hasCentral, backfillPlugins }) + // The catalog's client descriptors (claude/codex) are the honest static + // proxy for both declared-target derivations: status cannot see the runtime + // backfill/attach registries without activating plugins, so "this enabled + // plugin is a client adapter" is read off the descriptors. backfill keys its + // markers by owning-plugin name, attach by client name (the handlers' + // request keys) — buildClientActionsReport derives both from the one map. + clientActions = buildClientActionsReport({ status: actionStatus, config, hasCentral, clientDescriptors }) } catch { /* best-effort probe */ } // ----- recent errors ----- @@ -557,27 +556,38 @@ export async function collectHypAwareStatus(opts = {}) { * Per-provider state: * - `done` / `failed` come straight from a persisted marker (any request * key, even one whose plugin has since left the config). - * - `pending` / `n/a` are derived for *declared* backfill targets — a - * plugin entry carrying its own `config.backfill` block. Backfill - * capability is a runtime fact (a registered `BackfillContribution`, - * LLP 0041 §per-plugin-capability) the status collector cannot see - * without activating plugins, so the declared policy is the honest, - * provider-agnostic signal: `on_join: false` or a non-joined host → - * `n/a` (the reconciler is a no-op); otherwise desired-but-unrun → - * `pending`. + * - `pending` / `n/a` are derived for *declared* targets the reconciler would + * act on but has not yet. Two handlers declare such targets: + * - **backfill** (LLP 0037) — a plugin entry's `config.backfill` block, + * keyed by owning-plugin name. + * - **attach** (LLP 0044 / 0045) — an enabled client adapter, keyed by + * *client* name (the attach handler's request key), opted out by + * `config.attach.on_join: false`. + * Neither capability is visible to the status collector without activating + * plugins (both are runtime registrations — LLP 0041 §per-plugin-capability), + * so the catalog's client descriptors are the honest, provider-agnostic + * proxy: `on_join: false` or a non-joined host → `n/a` (the reconciler is a + * no-op); otherwise desired-but-unrun → `pending`. * - * @param {{ status: ClientActionStatus, config: HypAwareV2Config | null, hasCentral: boolean, backfillPlugins?: Set }} args + * @param {{ status: ClientActionStatus, config: HypAwareV2Config | null, hasCentral: boolean, clientDescriptors?: Map }} args * @returns {ClientActionsReport | null} * @ref LLP 0041#idempotency-and-completion-state [implements] — per-provider done/failed/pending/n-a derived from the per-handler/per-request-key marker store, no reconcile pass */ -function buildClientActionsReport({ status, config, hasCentral, backfillPlugins }) { +function buildClientActionsReport({ status, config, hasCentral, clientDescriptors }) { /** @type {ClientActionReport[]} */ const actions = [] const byKind = status?.byKind ?? {} - const backfillCapable = backfillPlugins ?? new Set() + // Client-adapter plugins (claude/codex), derived statically from the catalog + // descriptors — the set the backfill default-on derivation needs ("this + // enabled plugin imports on join") and, via the descriptors themselves, the + // universe of attach targets below. + const clientAdapterPlugins = new Set( + [...(clientDescriptors?.values() ?? [])].map((d) => d.plugin) + ) // Declared backfill targets: enabled plugin entries that drive - // backfill-on-join (LLP 0037 — policy rides the owning plugin). Two cases: + // backfill-on-join (LLP 0037 — policy rides the owning plugin). Keyed by + // owning-plugin name (the backfill handler's request key, LLP 0041). Two cases: // 1. An explicit `config.backfill` block (any host). // 2. *Default-on*: a known backfill provider with no explicit block — on // a joined host `backfillHandler.desired()` still emits for it, so it @@ -598,22 +608,60 @@ function buildClientActionsReport({ status, config, hasCentral, backfillPlugins // (block present, `on_join` absent) is default-on → not suppressed. const onJoin = readBackfillPolicy(entry).onJoin !== false declared.set(entry.name, { onJoin }) - } else if (hasCentral && backfillCapable.has(entry.name)) { + } else if (hasCentral && clientAdapterPlugins.has(entry.name)) { declared.set(entry.name, { onJoin: true }) } } - // Kinds to render: every kind the markers record, plus `backfill` when - // any target is declared (so a configured-but-unrun target still shows). + // Declared attach targets (LLP 0044 / 0045): symmetric to backfill, but keyed + // by *client* name — the attach handler's request key is the client name + // (`descriptor.name`), not the owning plugin — so a `done` attach marker the + // handler writes merges with the declared target instead of doubling it. Every + // enabled client adapter on a joined host is a desired attach target by + // default; an explicit `config.attach` block opts out via `on_join: false`, + // read through the shared `readAttachPolicy` (the `backfill_policy.js` twin) so + // status can never disagree with `action_attach.js` about what a block means. + // The default-on case is gated on `hasCentral` for the same V1-surface reason + // as backfill: a bare local claude/codex install shows nothing. + // @ref LLP 0044#status-surface [implements] — per-client done/failed/pending/n-a; `on_join:false` or non-joined → n/a, never degrading + /** @type {Map} */ + const enabledByPlugin = new Map() + for (const entry of config?.plugins ?? []) { + if (entry.enabled === false) continue + enabledByPlugin.set(entry.name, entry) + } + /** @type {Map} */ + const declaredAttach = new Map() + for (const [clientName, descriptor] of clientDescriptors ?? new Map()) { + const entry = enabledByPlugin.get(descriptor.plugin) + if (!entry) continue + const raw = entry.config?.attach + const hasBlock = !!raw && typeof raw === 'object' && !Array.isArray(raw) + if (hasBlock) { + const onJoin = readAttachPolicy(entry).onJoin !== false + declaredAttach.set(clientName, { onJoin }) + } else if (hasCentral) { + declaredAttach.set(clientName, { onJoin: true }) + } + } + + // Kinds to render: every kind the markers record, plus a kind for each + // handler that declared a target (so a configured-but-unrun target shows even + // with no marker yet). `backfill` keys by plugin, `attach` by client name. + /** @type {Record>} */ + const declaredByKind = { backfill: declared, attach: declaredAttach } /** @type {Set} */ const kinds = new Set(Object.keys(byKind)) - if (declared.size > 0) kinds.add('backfill') + for (const [k, m] of Object.entries(declaredByKind)) { + if (m.size > 0) kinds.add(k) + } for (const kind of [...kinds].sort()) { const markers = byKind[kind] ?? {} + const declaredForKind = declaredByKind[kind] /** @type {Set} */ const keys = new Set(Object.keys(markers)) - if (kind === 'backfill') for (const name of declared.keys()) keys.add(name) + if (declaredForKind) for (const name of declaredForKind.keys()) keys.add(name) for (const requestKey of [...keys].sort()) { const marker = markers[requestKey] if (marker && marker.status === 'failed') { @@ -626,7 +674,8 @@ function buildClientActionsReport({ status, config, hasCentral, backfillPlugins ...(typeof marker.attempts === 'number' ? { attempts: marker.attempts } : {}), }) } else if (marker) { - // `done` (run-once) or `applied` (reversible) — the effect is in place. + // `done` (run-once / attached) or `applied` (reversible) — the effect + // is in place. For attach a `done` marker is the "attached" rendering. actions.push({ kind, requestKey, @@ -635,10 +684,10 @@ function buildClientActionsReport({ status, config, hasCentral, backfillPlugins ...(typeof marker.at === 'string' ? { at: marker.at } : {}), }) } else { - // No marker: a declared backfill target. Suppressed (on_join:false) - // or inert (host never joined → the reconciler is a no-op) → n/a; - // otherwise desired and simply not run yet → pending. - const decl = declared.get(requestKey) + // No marker: a declared backfill or attach target. Suppressed + // (on_join:false) or inert (host never joined → the reconciler is a + // no-op) → n/a; otherwise desired and simply not run yet → pending. + const decl = declaredForKind?.get(requestKey) const suppressed = decl ? !decl.onJoin : false const state = suppressed || !hasCentral ? 'n/a' : 'pending' actions.push({ kind, requestKey, state }) diff --git a/test/core/status-client-actions.test.js b/test/core/status-client-actions.test.js index 9ddecf3..2f80048 100644 --- a/test/core/status-client-actions.test.js +++ b/test/core/status-client-actions.test.js @@ -101,10 +101,20 @@ test('mixed done/failed/pending/n-a reads cleanly off the marker store + config' assert.equal(m.get('@acme/failed-plugin')?.attempts, 2) assert.equal(m.get('@hypaware/claude')?.state, 'pending') - assert.equal(m.get('@hypaware/codex')?.state, 'n/a') // on_join:false → suppressed - - // Every entry is namespaced under the backfill handler kind. - assert.ok(report.clientActions.actions.every((a) => a.kind === 'backfill')) + assert.equal(m.get('@hypaware/codex')?.state, 'n/a') // backfill on_join:false → suppressed + + // The backfill rows are namespaced under the backfill handler kind, keyed by + // owning-plugin name; the only other kind present is `attach` (T9 — the + // joined claude/codex client adapters are default-on attach targets with no + // marker, so they surface `pending`, keyed by *client* name). + const backfillRows = report.clientActions.actions.filter((a) => a.kind === 'backfill') + assert.ok(backfillRows.length >= 4) // done, failed, claude, codex + assert.ok(report.clientActions.actions.every((a) => a.kind === 'backfill' || a.kind === 'attach')) + const attach = new Map( + report.clientActions.actions.filter((a) => a.kind === 'attach').map((a) => [a.requestKey, a]) + ) + assert.equal(attach.get('claude')?.state, 'pending') + assert.equal(attach.get('codex')?.state, 'pending') // no `attach` block → default-on }) test('a malformed on_join block renders n/a (not pending) on a joined host', async () => { @@ -279,3 +289,166 @@ test('text renderer prints the client actions section with per-state detail', as assert.match(text, /backfill @hypaware\/claude\s+\[pending\]/) assert.match(text, /backfill @hypaware\/codex\s+\[n\/a\]/) }) + +// T9 — the declared-attach-targets derivation (LLP 0044 / 0045), symmetric to +// backfill but keyed by *client* name (the attach handler's request key). Status +// reads the marker file and never runs a pass; a failed/pending attach is loud +// but never flips `overall` to `degraded`. +// @ref LLP 0044#status-surface [tests] + +/** @param {ClientActionReport[]} actions */ +function attachByKey(actions) { + /** @type {Map} */ + const m = new Map() + for (const a of actions) if (a.kind === 'attach') m.set(a.requestKey, a) + return m +} + +test('attach declared targets read mixed done/failed/pending/n-a cleanly (T9)', async () => { + const hypHome = await makeHome() + const stateRoot = path.join(hypHome, 'hypaware') + + // Joined host: central enables the gateway plus both client adapters. claude + // is default-on (no `attach` block) → pending until a pass runs; codex opts + // out (`attach.on_join: false`) → n/a. + const seedPath = centralSeedPath(stateRoot) + await fs.mkdir(path.dirname(seedPath), { recursive: true }) + await fs.writeFile(seedPath, JSON.stringify({ + version: 2, + plugins: [ + { name: '@hypaware/central' }, + { name: '@hypaware/ai-gateway' }, + { name: '@hypaware/claude' }, + { name: '@hypaware/codex', config: { attach: { on_join: false } } }, + ], + sinks: { central: { plugin: '@hypaware/central', config: {} } }, + }) + '\n') + await fs.writeFile(defaultConfigPath(hypHome), JSON.stringify({ version: 2, plugins: [] }) + '\n') + + // Attach markers from a prior pass, keyed by CLIENT name (the attach handler's + // request key). A done (= attached) and a failed entry, even for a client + // whose descriptor is not in the catalog — markers surface regardless. + await writeMarkers(hypHome, { + attach: { + 'acme-done': { status: 'done', request_key: 'acme-done', at: '2026-06-25T00:00:00.000Z' }, + 'acme-failed': { status: 'failed', request_key: 'acme-failed', reason: 'settings not writable', last_attempt: '2026-06-25T01:00:00.000Z', attempts: 2 }, + }, + }) + + const report = await collectHypAwareStatus({ env: env(hypHome) }) + assert.ok(report.clientActions, 'clientActions populated') + const attach = attachByKey(report.clientActions.actions) + + assert.equal(attach.get('acme-done')?.state, 'done') + assert.equal(attach.get('acme-done')?.at, '2026-06-25T00:00:00.000Z') + assert.equal(attach.get('acme-failed')?.state, 'failed') + assert.equal(attach.get('acme-failed')?.reason, 'settings not writable') + assert.equal(attach.get('acme-failed')?.attempts, 2) + assert.equal(attach.get('claude')?.state, 'pending') // default-on, no marker + assert.equal(attach.get('codex')?.state, 'n/a') // attach.on_join:false → suppressed + + // The generic done/failed/pending/n-a rendering also reaches the text surface. + const stdout = makeBuf() + renderStatusText({ report, clientNames: [], datasets: [], cacheRoot: '/tmp/cache', stdout }) + const text = stdout.text() + assert.match(text, /attach acme-done\s+\[done\]/) + assert.match(text, /attach claude\s+\[pending\]/) + assert.match(text, /attach codex\s+\[n\/a\]/) +}) + +test('attach renders n/a for on_join:false and for a non-joined explicit target; bare local stays V1 (T9)', async () => { + // (a) joined host, explicit opt-out → n/a. + const home1 = await makeHome() + const seed1 = centralSeedPath(path.join(home1, 'hypaware')) + await fs.mkdir(path.dirname(seed1), { recursive: true }) + await fs.writeFile(seed1, JSON.stringify({ + version: 2, + plugins: [ + { name: '@hypaware/central' }, + { name: '@hypaware/ai-gateway' }, + { name: '@hypaware/claude', config: { attach: { on_join: false } } }, + ], + sinks: { central: { plugin: '@hypaware/central', config: {} } }, + }) + '\n') + await fs.writeFile(defaultConfigPath(home1), JSON.stringify({ version: 2, plugins: [] }) + '\n') + const r1 = await collectHypAwareStatus({ env: env(home1) }) + assert.equal(attachByKey(r1.clientActions?.actions ?? []).get('claude')?.state, 'n/a') + + // (b) non-joined host, *explicit* attach block → n/a: the reconciler never + // runs off a joined host, so even an opted-in target is inert. + const home2 = await makeHome() + await fs.writeFile(defaultConfigPath(home2), JSON.stringify({ + version: 2, + plugins: [ + { name: '@hypaware/ai-gateway' }, + { name: '@hypaware/claude', config: { attach: { on_join: true } } }, + ], + }) + '\n') + const r2 = await collectHypAwareStatus({ env: env(home2) }) + assert.equal(attachByKey(r2.clientActions?.actions ?? []).get('claude')?.state, 'n/a') + + // (c) non-joined host, NO attach block → V1 surface unchanged (no attach row). + const home3 = await makeHome() + await fs.writeFile(defaultConfigPath(home3), JSON.stringify({ + version: 2, + plugins: [{ name: '@hypaware/ai-gateway' }, { name: '@hypaware/claude' }], + }) + '\n') + const r3 = await collectHypAwareStatus({ env: env(home3) }) + assert.equal(attachByKey(r3.clientActions?.actions ?? []).size, 0) +}) + +test('a failed attach does not flip overall to degraded (T9)', async () => { + const hypHome = await makeHome() + // Minimal otherwise-healthy config (gateway only) so the only notable state + // is the failed attach marker. + await fs.writeFile(defaultConfigPath(hypHome), JSON.stringify({ + version: 2, + plugins: [{ name: '@hypaware/ai-gateway' }], + }) + '\n') + await writeMarkers(hypHome, { + attach: { + claude: { status: 'failed', request_key: 'claude', reason: 'boom', last_attempt: '2026-06-25T01:00:00.000Z', attempts: 3 }, + }, + }) + + const report = await collectHypAwareStatus({ env: env(hypHome) }) + assert.equal(report.overall, 'healthy') + assert.ok(report.clientActions) + assert.equal(attachByKey(report.clientActions?.actions ?? []).get('claude')?.state, 'failed') + // The failure is its own section, never a degrading diagnostic. + assert.ok(!report.diagnostics.some((d) => d.message.includes('boom'))) +}) + +test('a done attach marker renders attached and collapses with the declared target — no double row (T9)', async () => { + const hypHome = await makeHome() + const stateRoot = path.join(hypHome, 'hypaware') + const seedPath = centralSeedPath(stateRoot) + await fs.mkdir(path.dirname(seedPath), { recursive: true }) + await fs.writeFile(seedPath, JSON.stringify({ + version: 2, + plugins: [ + { name: '@hypaware/central' }, + { name: '@hypaware/ai-gateway' }, + { name: '@hypaware/claude' }, + ], + sinks: { central: { plugin: '@hypaware/central', config: {} } }, + }) + '\n') + await fs.writeFile(defaultConfigPath(hypHome), JSON.stringify({ version: 2, plugins: [] }) + '\n') + + // claude is a declared (default-on) attach target AND has a done marker keyed + // by client name. The two must collapse into a single `done` row — proving the + // declared-target key matches the handler's request key (the client name), so + // the marker is not double-counted as both done and pending. + await writeMarkers(hypHome, { + attach: { + claude: { status: 'done', request_key: 'claude', at: '2026-06-25T00:00:00.000Z' }, + }, + }) + + const report = await collectHypAwareStatus({ env: env(hypHome) }) + const claudeRows = (report.clientActions?.actions ?? []) + .filter((a) => a.kind === 'attach' && a.requestKey === 'claude') + assert.equal(claudeRows.length, 1, 'exactly one attach row for claude (no done+pending double)') + assert.equal(claudeRows[0]?.state, 'done') + assert.equal(claudeRows[0]?.at, '2026-06-25T00:00:00.000Z') +}) From 0b4b90f55ae3742eae44dc9089e4bb4f49a3784c Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:47:25 -0700 Subject: [PATCH 08/15] Attach handler: action_attach.js (createAttachHandler + attachHandler) (LLP 0044/0045 T6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reversible instance of the client-action reconciler — the action_backfill.js twin and the first handler to implement reverse(). New src/core/config/action_attach.js exporting createAttachHandler(opts) and the default attachHandler: - desired() iterates ctx.clientDescriptors ∩ enabled plugins ∩ readAttachPolicy (the attach_policy.js opt-out), guarded on ctx.clients.getClient(name) being present so it never names a client perform() can't reach. Daemon-only by construction: inert when clientDescriptors or clients is absent (a plain CLI boot). - perform() is in-process (a bounded settings write, not a subprocess): resolves the runtime registration, calls attach({ endpoint: ctx.endpoint, config:{}, stdout, stderr, json:true }), parses the one-line JSON, and records the marker detail (settings_path, prev_value). A throw becomes a failed outcome the reconciler retries. - reverse() is disk-driven, not adapter-driven: it runs after the staged restart has unloaded the adapter, so it replays the single core undo (detachClientFromDisk) from the descriptor's attachProbe + the self-describing marker, never touching ctx.clients. Adds the attach handler's type surface (ClientDetachFromDisk seam + CreateAttachHandlerOptions) to config/types.d.ts, the direct analog of CreateBackfillHandlerOptions. Unit-tested (22 cases) with injected fake clientDescriptors + clients + fs, including a real fs round-trip proving reverse runs with no adapter loaded. The reconciler is left untouched (threading the new context fields onto ctx is T7's daemon wiring). Task-Id: T6 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/config/action_attach.js | 302 +++++++++++++++++++ src/core/config/types.d.ts | 30 ++ test/core/action-attach.test.js | 478 +++++++++++++++++++++++++++++++ 3 files changed, 810 insertions(+) create mode 100644 src/core/config/action_attach.js create mode 100644 test/core/action-attach.test.js diff --git a/src/core/config/action_attach.js b/src/core/config/action_attach.js new file mode 100644 index 0000000..62b1a6a --- /dev/null +++ b/src/core/config/action_attach.js @@ -0,0 +1,302 @@ +// @ts-check + +import { Attr } from '../observability/index.js' +import { readAttachPolicy } from './attach_policy.js' +import { detachClientFromDisk } from './client_detach_disk.js' + +/** + * @import { + * ActionContext, + * ActionHandler, + * ActionOutcome, + * ClientDetachFromDisk, + * CreateAttachHandlerOptions, + * DesiredAction, + * } from './types.d.ts' + * @import { JsonObject } from '../../../collectivus-plugin-kernel-types.d.ts' + */ + +/** + * The attach action handler — the reversible instance of the generic + * client-action reconciler (LLP 0036 / LLP 0044). When a joined machine + * confirms a central config that enables a client adapter, the daemon performs + * that client's attach machine-effect (a bounded settings write, in-process — + * *not* a subprocess like backfill); when the config later drops the client it + * reverses it. It is the `action_backfill.js` twin, the first handler to + * implement `reverse()`. + * + * Three roles split across two seams the daemon threads onto the context + * (LLP 0045 §Part 1): `ctx.clientDescriptors` *enumerates* the client adapters + * and their owning plugins (for `desired()` and the disk-driven undo's + * `attachProbe`), while the runtime `ctx.clients` registry only *invokes* the + * effect (`getClient(name).attach(...)`). The registry carries no owning-plugin + * field, so descriptors are the source of truth for "is this client's plugin + * enabled?"; the registry is consulted only to reach `perform()`. + * + * `perform()` is adapter-driven (it needs a live `attach()`); `reverse()` is + * **disk-driven** — it runs after the staged restart has already unloaded the + * adapter, so `ctx.clients` no longer has the dropped client and there is no + * live `detach()` to call. The undo is the single core routine + * `detachClientFromDisk` (LLP 0045 §Part 3), injectable so tests assert it runs + * without a gateway. + * + * @param {CreateAttachHandlerOptions} [opts] + * @returns {ActionHandler} + * @ref LLP 0045#part-2--the-attach-handler-srccoreconfigaction_attachjs [implements] — createAttachHandler(opts) → ActionHandler { kind:'attach', desired/perform/reverse }, mirroring action_backfill.js + * @ref LLP 0044 — client attach on join (the instance this realizes) + */ +export function createAttachHandler(opts = {}) { + /** @type {ClientDetachFromDisk} */ + const detach = opts.detach ?? detachClientFromDisk + + return { + kind: 'attach', + + /** + * Enumerate the client adapters to attach. Pure: iterate + * `ctx.clientDescriptors`, keep each descriptor whose owning `plugin` is + * enabled in `ctx.config.plugins`, whose entry does not set + * `attach.on_join: false` (read via `attach_policy.js`, the + * `backfill_policy.js` twin), and whose client the runtime registry has + * (`ctx.clients.getClient(name)` defined) so it never names a client + * `perform()` cannot reach. The owning plugin comes from the descriptor, + * not from `listClients()` (which omits it). Daemon-only by construction: + * a plain CLI boot has neither `clientDescriptors` nor `clients`, so the + * handler stays inert. + * + * @param {ActionContext} ctx + * @returns {DesiredAction[]} + * @ref LLP 0045#part-2--the-attach-handler-srccoreconfigaction_attachjs [implements] — desired() over clientDescriptors ∩ enabled plugins ∩ attach_policy, guarded on the runtime registry having the client + * @ref LLP 0044#consent--join-implies-consent-default-on [constrained-by] — default-on; only `attach.on_join:false` in the locked central plugin entry opts out + */ + desired(ctx) { + const descriptors = ctx.clientDescriptors + const clients = ctx.clients + // Daemon-only: with no client catalog or no gateway registry there is + // nothing to attach (LLP 0045 §Part 1 — attach is daemon-only by + // construction). + if (!descriptors || !clients) return [] + + const activePlugins = ctx.config.plugins ?? [] + const byPluginName = new Map( + activePlugins + .filter((p) => p && typeof p.name === 'string') + .map((p) => [p.name, p]) + ) + + /** @type {DesiredAction[]} */ + const desired = [] + for (const descriptor of descriptors.values()) { + const entry = byPluginName.get(descriptor.plugin) + // Plugin absent from config or explicitly disabled → not a target. + if (!entry || entry.enabled === false) continue + // Default-on: only an explicit `on_join: false` opts out. + if (readAttachPolicy(entry).onJoin === false) continue + // Never name a client the runtime registry can't reach. + if (!clients.getClient(descriptor.name)) continue + desired.push({ + requestKey: descriptor.name, + params: { client: descriptor.name, plugin: descriptor.plugin }, + }) + } + return desired + }, + + /** + * Attach one client. In-process (a bounded settings write — LLP 0041 + * §Execution isolation), not a subprocess like backfill. Resolves the + * runtime registration, calls `attach({ endpoint, config:{}, stdout, + * stderr, json:true })`, parses the one-line JSON the adapter emits, and + * records `settings_path` / `prev_value` as the marker detail. A throw + * (file not writable, malformed settings) becomes a `failed` outcome the + * reconciler records and retries next pass. + * + * @param {DesiredAction} action + * @param {ActionContext} ctx + * @returns {Promise} + * @ref LLP 0045#part-2--the-attach-handler-srccoreconfigaction_attachjs [implements] — perform() calls attach(json:true), parses the one-line JSON, records the marker detail (settings_path, prev_value) + */ + async perform(action, ctx) { + const params = action.params ?? {} + const client = + typeof params.client === 'string' && params.client.length > 0 + ? params.client + : action.requestKey + if (typeof client !== 'string' || client.length === 0) { + return { status: 'failed', reason: 'attach action missing client name' } + } + + const registration = ctx.clients?.getClient(client) + if (!registration) { + return { status: 'failed', reason: `no registered client '${client}' to attach` } + } + const endpoint = ctx.endpoint + if (typeof endpoint !== 'string' || endpoint.length === 0) { + return { status: 'failed', reason: 'attach action missing gateway endpoint' } + } + + const stdout = captureStream() + const stderr = captureStream() + + ctx.log.info('client_action.attach_perform', { + [Attr.COMPONENT]: 'action-attach', + [Attr.OPERATION]: 'client_action.perform', + [Attr.PLUGIN]: typeof params.plugin === 'string' ? params.plugin : client, + client, + endpoint, + [Attr.STATUS]: 'ok', + }) + + try { + await registration.attach({ endpoint, config: {}, stdout, stderr, json: true }) + } catch (err) { + return { status: 'failed', reason: err instanceof Error ? err.message : String(err) } + } + + const parsed = parseAttachOutput(stdout.text()) + // No throw = the attach applied. An unparseable / detail-less payload + // still records `done` (mirroring backfill's "exit 0 is authoritative") + // rather than re-running a successful attach — we just can't attach the + // marker detail. + if (!parsed) return { status: 'done' } + + /** @type {JsonObject} */ + const detail = {} + if (typeof parsed.settings_path === 'string') detail.settings_path = parsed.settings_path + if (typeof parsed.prev_value === 'string') detail.prev_value = parsed.prev_value + return Object.keys(detail).length > 0 ? { status: 'done', detail } : { status: 'done' } + }, + + /** + * Reverse a previously-applied attach whose request key the config no + * longer names (the central config dropped the client, or flipped + * `attach.on_join` to false). **Disk-driven, not adapter-driven**: the + * headline reverse fires only after the staged restart has unloaded the + * adapter, so `ctx.clients.getClient(client)` is `undefined` and there is + * no live `detach()` to call. Instead it reads the descriptor's + * `attachProbe` + the settings-file marker (the self-describing undo + * record `attach()` wrote) and replays the single core undo + * (`detachClientFromDisk`) — the same one `hyp detach` uses. It needs + * `ctx.clientDescriptors` and the filesystem, **never** `ctx.clients`. + * + * @param {string} requestKey The client name whose attach to reverse. + * @param {ActionContext} ctx + * @returns {Promise} + * @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [implements] — reverse() invokes the single disk-driven core undo (detachClientFromDisk), not ctx.clients + */ + async reverse(requestKey, ctx) { + const descriptor = ctx.clientDescriptors?.get(requestKey) + if (!descriptor) { + // The descriptor normally survives a fleet-drop (only the config entry + // goes away), so this is a real gap; keep the marker and retry. + return { status: 'failed', reason: `no client descriptor for '${requestKey}' to reverse` } + } + + ctx.log.info('client_action.attach_reverse', { + [Attr.COMPONENT]: 'action-attach', + [Attr.OPERATION]: 'client_action.reverse', + [Attr.PLUGIN]: descriptor.plugin, + client: descriptor.name, + [Attr.STATUS]: 'ok', + }) + + let result + try { + result = await detach({ descriptor, env: ctx.env }) + } catch (err) { + return { status: 'failed', reason: err instanceof Error ? err.message : String(err) } + } + + if (result.warning) { + // The managed value was overridden externally after we attached; the + // undo left it in place. Surface it, but the undo itself succeeded. + ctx.log.warn('client_action.attach_reverse_warning', { + [Attr.COMPONENT]: 'action-attach', + [Attr.OPERATION]: 'client_action.reverse', + client: descriptor.name, + [Attr.STATUS]: 'ok', + detail: result.warning, + }) + } + + // Idempotent: a no-op (file already clean / marker absent) is still a + // successful undo — the reconciler drops the marker either way. + return { status: 'done' } + }, + } +} + +/** + * The default `attachHandler` the daemon registers the reconciler with — first + * in the `[attachHandler, backfillHandler]` order so in-process live-capture + * wiring starts ahead of the (possibly multi-minute) backfill subprocess + * (LLP 0045 §Part 7). Uses the real `detachClientFromDisk`; tests build their + * own via {@link createAttachHandler} with an injected `detach`. + * + * @type {ActionHandler} + */ +export const attachHandler = createAttachHandler() + +/* ------------------------------- Internals ------------------------------- */ + +/** + * A capturing `WriteStream` — accumulates every `write(chunk)` so the handler + * can parse the adapter's machine-readable `json: true` output after the + * in-process `attach()` returns. (The real CLI hands the adapter `ctx.stdout`; + * the handler instead captures it.) + * + * @returns {{ write(chunk: string): boolean, text(): string }} + */ +function captureStream() { + /** @type {string[]} */ + const chunks = [] + return { + write(chunk) { + chunks.push(String(chunk)) + return true + }, + text() { + return chunks.join('') + }, + } +} + +/** + * Parse the one-line JSON object an adapter emits in `json: true` mode + * (`{ status, action, client, dry_run, settings_path?, port?, changed?, + * prev_value? }`). Tolerant: trims, and on a parse miss falls back to the last + * non-empty line (in case prose leaked onto stdout). Returns `undefined` when + * nothing parses to an object so the caller records `done` without detail + * rather than re-running a successful attach. + * + * @param {string} stdout + * @returns {Record | undefined} + */ +function parseAttachOutput(stdout) { + const trimmed = stdout.trim() + if (trimmed.length === 0) return undefined + + let parsed = tryParseObject(trimmed) + if (parsed === undefined) { + const lines = trimmed.split('\n').map((l) => l.trim()).filter((l) => l.length > 0) + const last = lines[lines.length - 1] + if (last !== undefined) parsed = tryParseObject(last) + } + return parsed +} + +/** + * @param {string} text + * @returns {Record | undefined} + */ +function tryParseObject(text) { + let value + try { + value = JSON.parse(text) + } catch { + return undefined + } + return value !== null && typeof value === 'object' && !Array.isArray(value) + ? /** @type {Record} */ (value) + : undefined +} diff --git a/src/core/config/types.d.ts b/src/core/config/types.d.ts index 791fbb0..31c3aa2 100644 --- a/src/core/config/types.d.ts +++ b/src/core/config/types.d.ts @@ -14,6 +14,7 @@ import type { AiGatewayCapability, } from '../../../collectivus-plugin-kernel-types.d.ts' import type { ClientDescriptor } from '../plugin_catalog.js' +import type { DetachFromDiskResult } from './client_detach_disk.js' /** * Outcome of the `init` overwrite guard (LLP 0031). `proceed` is true @@ -561,4 +562,33 @@ export interface CreateBackfillHandlerOptions { log?: PluginLogger } +// ============================================================================= +// Attach action handler (LLP 0044 / LLP 0045 Part 2) +// ============================================================================= + +/** + * The disk-driven undo seam the attach handler's `reverse()` invokes — the + * single core detach (`detachClientFromDisk`, LLP 0045 §Part 3). Injected in + * tests so `reverse()` can be exercised against a fixture / fake without a live + * gateway; the default is the real `detachClientFromDisk`. The seam only needs + * the fields the handler passes (`descriptor` + the daemon-resolved `env`); the + * real implementation accepts more (an injectable `fs` / `homeDir`), so it is + * assignable to this narrower type. + */ +export type ClientDetachFromDisk = (args: { + descriptor: ClientDescriptor + homeDir?: string + env?: NodeJS.ProcessEnv +}) => Promise + +export interface CreateAttachHandlerOptions { + /** + * The disk-driven undo seam `reverse()` calls; defaults to the real + * `detachClientFromDisk`. Injected in tests to assert the undo runs without + * touching `ctx.clients` (which lacks the dropped client at reverse time). + */ + detach?: ClientDetachFromDisk + log?: PluginLogger +} + export type { ConfigStageResult, ConfigApplyErrorKind } diff --git a/test/core/action-attach.test.js b/test/core/action-attach.test.js new file mode 100644 index 0000000..ce7c63a --- /dev/null +++ b/test/core/action-attach.test.js @@ -0,0 +1,478 @@ +// @ts-check + +import test from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' + +import { createAttachHandler, attachHandler } from '../../src/core/config/action_attach.js' +import { detachClientFromDisk } from '../../src/core/config/client_detach_disk.js' + +/** + * T6 (LLP 0044/0045/0046): the attach action handler — the reversible + * instance of the client-action reconciler, the `action_backfill.js` twin. + * These tests drive the handler hooks directly with injected fake + * `clientDescriptors` + `clients` + filesystem, the way the plan prescribes; + * the reconciler's generic gap loop is covered by `action-reconciler.test.js`. + * + * @import { ActionContext, ActionHandler } from '../../src/core/config/types.d.ts' + * @import { ClientDescriptor } from '../../src/core/plugin_catalog.js' + */ + +/** + * Narrow the optional `reverse?` hook to a defined function (attach is the + * reversible handler, so it always implements it). + * @param {ActionHandler} handler + * @returns {NonNullable} + */ +function reverseOf(handler) { + assert.ok(handler.reverse, 'attach handler must implement reverse()') + return handler.reverse +} + +/** A quiet logger so tests don't spam stderr. */ +const NOOP_LOG = { debug() {}, info() {}, warn() {}, error() {} } + +const FIXED_NOW = Date.parse('2026-06-25T00:00:00.000Z') +const ENDPOINT = 'http://127.0.0.1:4123' + +/** @type {ClientDescriptor} */ +const CLAUDE_DESCRIPTOR = { + plugin: /** @type {any} */ ('@hypaware/claude'), + name: 'claude', + skillDir: 'skills/claude', + attachProbe: { format: 'json', settings_file: '.claude/settings.json', marker_key: '_hypaware' }, +} + +/** @type {ClientDescriptor} */ +const CODEX_DESCRIPTOR = { + plugin: /** @type {any} */ ('@hypaware/codex'), + name: 'codex', + skillDir: 'skills/codex', + attachProbe: { format: 'toml', settings_file: '.codex/config.toml', marker_header: '[model_providers.hypaware]' }, +} + +/** + * @param {ClientDescriptor[]} list + * @returns {Map} + */ +function descriptorMap(list) { + return new Map(list.map((d) => [d.name, d])) +} + +/** + * A fake gateway registry over a fixed set of registered client adapters. Only + * `getClient` / `listClients` are exercised; the rest satisfy the shape. + * @param {Record} registrations client name -> registration + * @returns {any} + */ +function clientsWith(registrations) { + const map = new Map(Object.entries(registrations)) + return { + getClient(/** @type {string} */ name) { return map.get(name) }, + listClients() { return [...map.values()] }, + registerClient() {}, + registerUpstreamPreset() {}, + registerExchangeProjector() {}, + registerSettlementEnricher() {}, + localEndpoint() { return ENDPOINT }, + } +} + +/** + * A fake client registration whose `attach()` writes the adapter's one-line + * `json: true` payload (or throws / emits prose, per opts). + * @param {string} name + * @param {{ payload?: any, prose?: string, throws?: Error, onAttach?: (ctx: any) => void }} [opts] + * @returns {any} + */ +function attachRegistration(name, opts = {}) { + return { + name, + defaultUpstream: 'anthropic', + /** @param {any} ctx */ + async attach(ctx) { + opts.onAttach?.(ctx) + if (opts.throws) throw opts.throws + if (typeof opts.prose === 'string') { + ctx.stdout.write(opts.prose) + return + } + const payload = opts.payload ?? { + status: 'attached', action: 'attach', client: name, dry_run: false, changed: true, + } + ctx.stdout.write(JSON.stringify(payload)) + }, + } +} + +/** + * Build the ActionContext a handler hook receives. + * @param {{ + * plugins?: any[], + * descriptors?: Map, + * clients?: any, + * endpoint?: string | undefined, + * env?: NodeJS.ProcessEnv, + * }} [opts] + * @returns {ActionContext} + */ +function makeCtx(opts = {}) { + return { + config: /** @type {any} */ ({ version: 2, plugins: opts.plugins ?? [] }), + backfills: /** @type {any} */ ({ register() {}, get() { return undefined }, list() { return [] } }), + env: opts.env ?? { ...process.env }, + clientDescriptors: opts.descriptors, + clients: /** @type {any} */ (opts.clients), + endpoint: 'endpoint' in opts ? opts.endpoint : ENDPOINT, + now: () => FIXED_NOW, + log: NOOP_LOG, + } +} + +/* -------------------------------- shape --------------------------------- */ + +test('the default attachHandler is an attach-kind, reversible ActionHandler', () => { + assert.equal(attachHandler.kind, 'attach') + assert.equal(typeof attachHandler.desired, 'function') + assert.equal(typeof attachHandler.perform, 'function') + // Unlike backfill (run-once), attach implements reverse(). + assert.equal(typeof attachHandler.reverse, 'function') +}) + +/* ------------------------------- desired() ------------------------------- */ + +test('desired() emits one action per enabled client descriptor with a registered client', () => { + const handler = createAttachHandler() + const desired = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: {} }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(desired, [ + { requestKey: 'claude', params: { client: 'claude', plugin: '@hypaware/claude' } }, + ]) +}) + +test('desired() emits an action per enabled descriptor across two client plugins', () => { + const handler = createAttachHandler() + const desired = handler.desired(makeCtx({ + plugins: [ + { name: '@hypaware/claude', enabled: true, config: {} }, + { name: '@hypaware/codex', enabled: true, config: {} }, + ], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR, CODEX_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude'), codex: attachRegistration('codex') }), + })) + assert.deepEqual(desired.map((d) => d.requestKey).sort(), ['claude', 'codex']) +}) + +test('desired() excludes a descriptor whose owning plugin is disabled or absent', () => { + const handler = createAttachHandler() + const disabled = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: false, config: {} }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(disabled, []) + const absent = handler.desired(makeCtx({ + plugins: [], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(absent, []) +}) + +test('desired() honors an explicit attach.on_join:false opt-out (no action)', () => { + const handler = createAttachHandler() + const desired = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: { attach: { on_join: false } } }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(desired, []) +}) + +test('desired() does not fail open on a non-boolean on_join (treats it as opt-out)', () => { + const handler = createAttachHandler() + // The typo'd JSON string `"false"` is not a boolean; it must suppress, not + // fall through to default-on and silently edit the user's settings file. + const stringFalse = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: { attach: { on_join: 'false' } } }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(stringFalse, [], 'on_join:"false" (string) must not attach') +}) + +test('desired() guards on the runtime registry actually having the client', () => { + const handler = createAttachHandler() + // Enabled plugin + descriptor, but the gateway registered no such client → + // never name a client `perform()` cannot reach. + const desired = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: {} }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({}), + })) + assert.deepEqual(desired, []) +}) + +test('desired() is daemon-only: inert with no clientDescriptors and with no clients (a plain CLI boot)', () => { + const handler = createAttachHandler() + // No client catalog at all. + assert.deepEqual(handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: {} }], + descriptors: undefined, + clients: clientsWith({ claude: attachRegistration('claude') }), + })), []) + // Descriptors but no gateway registry (gateway capability absent). + assert.deepEqual(handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: {} }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: undefined, + })), []) +}) + +/* ------------------------------- perform() ------------------------------- */ + +test('perform() attaches via the registry (endpoint + json mode) and records settings_path + prev_value', async () => { + /** @type {any} */ + let attachCtx + const registration = attachRegistration('claude', { + onAttach: (ctx) => { attachCtx = ctx }, + payload: { + status: 'attached', action: 'attach', client: 'claude', dry_run: false, + changed: true, settings_path: '/home/u/.claude/settings.json', port: 4123, + prev_value: 'https://foreign.example/api', + }, + }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude', plugin: '@hypaware/claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.deepEqual(outcome, { + status: 'done', + detail: { settings_path: '/home/u/.claude/settings.json', prev_value: 'https://foreign.example/api' }, + }) + // The adapter was invoked with the gateway endpoint, an empty config, and + // the machine-readable json flag. + assert.equal(attachCtx.endpoint, ENDPOINT) + assert.equal(attachCtx.json, true) + assert.deepEqual(attachCtx.config, {}) +}) + +test('perform() records done with only settings_path when the attach had no prior value to back up', async () => { + const registration = attachRegistration('claude', { + payload: { + status: 'attached', action: 'attach', client: 'claude', dry_run: false, + changed: true, settings_path: '/home/u/.claude/settings.json', + }, + }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.deepEqual(outcome, { status: 'done', detail: { settings_path: '/home/u/.claude/settings.json' } }) +}) + +test('perform() records done (no detail) on an idempotent re-attach (changed:false)', async () => { + const registration = attachRegistration('claude', { + payload: { status: 'noop', action: 'attach', client: 'claude', dry_run: false, changed: false }, + }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.deepEqual(outcome, { status: 'done' }) +}) + +test('perform() records done (no detail) when the adapter emits an unparseable payload', async () => { + const registration = attachRegistration('claude', { prose: 'attached claude (human prose)\n' }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.deepEqual(outcome, { status: 'done' }) +}) + +test('perform() parses the last non-empty line when prose precedes the JSON', async () => { + const registration = attachRegistration('claude', { + onAttach: (ctx) => { + ctx.stdout.write('Attaching claude...\n') + ctx.stdout.write(JSON.stringify({ status: 'attached', client: 'claude', settings_path: '/p' }) + '\n') + }, + prose: '', + }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.deepEqual(outcome, { status: 'done', detail: { settings_path: '/p' } }) +}) + +test('perform() returns failed when the adapter throws (file not writable)', async () => { + const registration = attachRegistration('claude', { throws: new Error('EACCES: permission denied') }) + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: registration }) }), + ) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /EACCES/) +}) + +test('perform() returns failed when the registry has no such client', async () => { + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({}) }), + ) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /no registered client/) +}) + +test('perform() returns failed when no gateway endpoint is set', async () => { + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: 'claude', params: { client: 'claude' } }, + makeCtx({ clients: clientsWith({ claude: attachRegistration('claude') }), endpoint: undefined }), + ) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /endpoint/) +}) + +test('perform() guards against a missing client name', async () => { + const handler = createAttachHandler() + const outcome = await handler.perform( + { requestKey: '', params: {} }, + makeCtx({ clients: clientsWith({}) }), + ) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /missing client name/) +}) + +/* ------------------------------- reverse() ------------------------------- */ + +test('reverse() invokes the disk-driven undo once and never consults ctx.clients', async () => { + /** @type {any[]} */ + const calls = [] + // A registry that explodes if the handler ever touches it — proving reverse + // is adapter-independent (the dropped client is gone after the restart). + const poisonClients = { + getClient() { throw new Error('reverse() must not consult ctx.clients') }, + listClients() { throw new Error('reverse() must not consult ctx.clients') }, + } + const handler = createAttachHandler({ + detach: async (args) => { + calls.push(args) + return { changed: true, settingsPath: '/home/u/.claude/settings.json', restoredValue: 'https://foreign.example/api' } + }, + }) + const outcome = await reverseOf(handler)('claude', makeCtx({ + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: poisonClients, + })) + assert.deepEqual(outcome, { status: 'done' }) + assert.equal(calls.length, 1) + // The T4 undo got the descriptor (its attachProbe is what the undo replays). + assert.equal(calls[0].descriptor, CLAUDE_DESCRIPTOR) +}) + +test('reverse() replays the real core undo from disk with no adapter loaded (fs round-trip)', async () => { + const home = await fs.mkdtemp(path.join(os.tmpdir(), 'hyp-action-attach-')) + try { + const settingsPath = path.join(home, '.claude', 'settings.json') + await fs.mkdir(path.dirname(settingsPath), { recursive: true }) + // A hand-written self-describing marker — what claude `attach()` records: + // the managed env value plus the prior base URL to restore (LLP 0045 §Part 3). + const original = JSON.stringify({ + env: { ANTHROPIC_API_KEY: 'sk-x', ANTHROPIC_BASE_URL: ENDPOINT }, + _hypaware: { + prev_base_url: 'https://foreign.example/api', + managed: { env: { ANTHROPIC_BASE_URL: ENDPOINT }, hooks: [] }, + }, + }, null, 2) + '\n' + await fs.writeFile(settingsPath, original) + + // No ctx.clients at all — the adapter is unloaded post-restart. Bind the + // fixture home through the injected (real) detach. + const handler = createAttachHandler({ + detach: ({ descriptor }) => detachClientFromDisk({ descriptor, homeDir: home }), + }) + const outcome = await reverseOf(handler)('claude', makeCtx({ + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: undefined, + })) + assert.deepEqual(outcome, { status: 'done' }) + + const after = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + assert.equal('_hypaware' in after, false, 'marker stripped') + assert.equal(after.env.ANTHROPIC_BASE_URL, 'https://foreign.example/api', 'prior base URL restored') + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('reverse() returns failed (retry next pass) when the descriptor is gone from the catalog', async () => { + const handler = createAttachHandler({ + detach: async () => { throw new Error('detach should not be called without a descriptor') }, + }) + const outcome = await reverseOf(handler)('claude', makeCtx({ + descriptors: descriptorMap([]), + clients: undefined, + })) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /no client descriptor/) +}) + +test('reverse() returns failed when the disk undo throws (concurrent edit)', async () => { + const handler = createAttachHandler({ + detach: async () => { throw new Error('CONCURRENT_EDIT') }, + }) + const outcome = await reverseOf(handler)('claude', makeCtx({ + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: undefined, + })) + assert.equal(outcome.status, 'failed') + assert.match(String(outcome.reason), /CONCURRENT_EDIT/) +}) + +/* ---------------------- desired/reverse compose (gap) -------------------- */ + +test('the reverse-gap contract: a dropped client falls out of desired() and reverse() then undoes it', async () => { + /** @type {any[]} */ + const calls = [] + const handler = createAttachHandler({ + detach: async (args) => { calls.push(args); return { changed: true, settingsPath: '/p' } }, + }) + // Joined + enabled → named by desired() (the reconciler would perform it). + const named = handler.desired(makeCtx({ + plugins: [{ name: '@hypaware/claude', enabled: true, config: {} }], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(named.map((d) => d.requestKey), ['claude']) + + // Central config drops the plugin → desired() omits it; the descriptor stays + // in the catalog, so the reconciler's reverse gap fires for the marker key. + const dropped = handler.desired(makeCtx({ + plugins: [], + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: clientsWith({ claude: attachRegistration('claude') }), + })) + assert.deepEqual(dropped, []) + + // reverse() undoes it from disk. + const outcome = await reverseOf(handler)('claude', makeCtx({ + descriptors: descriptorMap([CLAUDE_DESCRIPTOR]), + clients: undefined, + })) + assert.deepEqual(outcome, { status: 'done' }) + assert.equal(calls.length, 1) +}) From 02e1770974daa52e839a1800a352c4da06059217 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 14:55:53 -0700 Subject: [PATCH 09/15] Client attach: retire adapter detach(), reroute manual detach to the core undo (LLP 0045 T5) Drop `detach` from `AiGatewayClientRegistration` (kernel type) and from the claude/codex `registerClient()` calls, and point `runClientLifecycle`'s detach branch at the single T4 core disk-driven undo (`detachClientFromDisk`), resolved per client via its `clientDescriptor`. There is now exactly one undo, shared by manual `hyp detach` and the daemon reconciler's `reverse()`, so the two cannot drift. - collectivus-plugin-kernel-types.d.ts: remove `detach()` from `AiGatewayClientRegistration` and the orphaned `AiGatewayClientDetachContext`; refresh the registerClient/getClient doc comments to attach-only. - claude/codex adapters: drop the registration `detach` property and the now-unused `writeDetachOutput`/type import (the settings-file `detach()` writers stay as exported utilities). - ai-gateway api.js: `registerClient` now requires only `attach()`. - core_commands.js: new `detachClientViaCore` + `writeCoreDetachOutput` emit the `client.detach` span and a plugin-agnostic `done`/no-op output via the core undo; the descriptor map is built once per detach invocation. - Tests/smoke updated for the rerouted path: ai-gateway-api (attach-only validation), command-dispatch + integration (detach is the disk undo, HOME isolated), and the claude_attach_detach smoke prose. claude_attach_detach / client_attach_idempotent smokes stay green. Task-Id: T5 Co-Authored-By: Claude Opus 4.8 (1M context) --- collectivus-plugin-kernel-types.d.ts | 33 ++-- .../plugins-workspace/ai-gateway/src/api.js | 6 +- .../plugins-workspace/claude/src/index.js | 123 +------------- .../plugins-workspace/codex/src/index.js | 129 +------------- .../smoke/flows/claude_attach_detach.js | 4 +- src/core/cli/core_commands.js | 157 +++++++++++++++++- test/core/command-dispatch.test.js | 17 +- test/core/integration.test.js | 12 +- test/plugins/ai-gateway-api.test.js | 17 +- 9 files changed, 214 insertions(+), 284 deletions(-) diff --git a/collectivus-plugin-kernel-types.d.ts b/collectivus-plugin-kernel-types.d.ts index 2fac874..cb1cde9 100644 --- a/collectivus-plugin-kernel-types.d.ts +++ b/collectivus-plugin-kernel-types.d.ts @@ -1273,9 +1273,10 @@ export interface VerbRegistry { * - register upstream presets (`registerUpstreamPreset`) that own * routing — the gateway no longer has any hardcoded provider routing * such as Anthropic-header or `/v1/messages` matching; - * - register client attach/detach helpers (`registerClient`) so the - * shared `hyp attach`/`hyp detach` CLI can dispatch without coupling - * core to client-specific code; + * - register a client `attach()` helper (`registerClient`) so the + * shared `hyp attach` CLI can dispatch without coupling core to + * client-specific code (the reversing detach is a core disk-driven + * undo, not a per-adapter hook); * - register exchange projectors (`registerExchangeProjector`) that * turn a captured HTTP/SSE exchange into a normalized list of * conversation messages. The gateway expands the projector's output @@ -1304,8 +1305,8 @@ export interface AiGatewayCapability { /** * Look up a registered client by name. Returns `undefined` when no * adapter plugin has registered under that name. Used by the shared - * `hyp attach`/`hyp detach` command router to dispatch to the right - * adapter without coupling core to plugin-specific code. + * `hyp attach` command router to dispatch to the right adapter + * without coupling core to plugin-specific code. */ getClient(name: string): AiGatewayClientRegistration | undefined /** @@ -1369,11 +1370,19 @@ export interface AiGatewayEndpointOptions { upstream?: string } +/** + * An adapter owns only `attach()`. The reversing detach is the single + * core, disk-driven undo (`detachClientFromDisk`) that both the manual + * `hyp detach` command and the daemon reconciler's `reverse()` route + * through, so there is no per-adapter detach for the one undo to drift + * from. + * + * @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [constrained-by] — AiGatewayClientRegistration.detach is retired; the sole undo lives in core + */ export interface AiGatewayClientRegistration { name: string defaultUpstream: string attach(ctx: AiGatewayClientAttachContext): Promise - detach(ctx: AiGatewayClientDetachContext): Promise status?(ctx: AiGatewayClientStatusContext): Promise } @@ -1397,18 +1406,6 @@ export interface AiGatewayClientAttachContext { json?: boolean } -export interface AiGatewayClientDetachContext { - config: JsonObject - stdout: WriteStream - stderr: WriteStream - dryRun?: boolean - /** - * When true the adapter must emit machine-readable JSON on stdout - * instead of human prose. One JSON object per detach call. - */ - json?: boolean -} - export interface AiGatewayClientStatusContext { config: JsonObject } diff --git a/hypaware-core/plugins-workspace/ai-gateway/src/api.js b/hypaware-core/plugins-workspace/ai-gateway/src/api.js index 61a57d0..0ba4043 100644 --- a/hypaware-core/plugins-workspace/ai-gateway/src/api.js +++ b/hypaware-core/plugins-workspace/ai-gateway/src/api.js @@ -57,8 +57,10 @@ export function createAiGatewayApi(state) { if (typeof client.defaultUpstream !== 'string' || client.defaultUpstream.length === 0) { throw new TypeError(`registerClient '${client.name}': defaultUpstream is required`) } - if (typeof client.attach !== 'function' || typeof client.detach !== 'function') { - throw new TypeError(`registerClient '${client.name}': attach()/detach() are required`) + // An adapter owns only attach(); the reversing detach is the single + // core disk-driven undo (LLP 0045 §Part 3), not a per-adapter hook. + if (typeof client.attach !== 'function') { + throw new TypeError(`registerClient '${client.name}': attach() is required`) } state.clients.set(client.name, client) }, diff --git a/hypaware-core/plugins-workspace/claude/src/index.js b/hypaware-core/plugins-workspace/claude/src/index.js index 33e00ed..36c83f1 100644 --- a/hypaware-core/plugins-workspace/claude/src/index.js +++ b/hypaware-core/plugins-workspace/claude/src/index.js @@ -8,7 +8,7 @@ import { fileURLToPath } from 'node:url' import { Attr, getLogger, withSpan } from '../../../../src/core/observability/index.js' import { defaultConfigPath } from '../../../../src/core/config/schema.js' import { CLAUDE_CONFIG_SECTION, validateClaudeConfig } from './config.js' -import { attach, defaultSettingsPath, detach } from './settings.js' +import { attach, defaultSettingsPath } from './settings.js' import { anthropicUpstreamPreset, createClaudeExchangeProjector } from './projector.js' import { createClaudeBackfillProvider } from './backfill.js' import { createClaudeSettlementEnricher } from './settle.js' @@ -16,7 +16,7 @@ import { defaultSessionContextFile } from './session_context.js' import { runClaudeSessionContextHook } from './hook_command.js' /** - * @import { AiGatewayCapability, AiGatewayClientAttachContext, AiGatewayClientDetachContext, CommandRunContext, HypAwareV2Config, PluginActivationContext } from '../../../../collectivus-plugin-kernel-types.d.ts' + * @import { AiGatewayCapability, AiGatewayClientAttachContext, CommandRunContext, HypAwareV2Config, PluginActivationContext } from '../../../../collectivus-plugin-kernel-types.d.ts' */ const PLUGIN_NAME = '@hypaware/claude' @@ -53,16 +53,17 @@ export function claudeSessionContextFile(ctx) { * * Resolves the `hypaware.ai-gateway@^2.0.0` capability, registers * the Anthropic upstream preset (path + header signature match) and - * the full Anthropic exchange projector, wires attach/detach against + * the full Anthropic exchange projector, wires `attach()` against * `~/.claude/settings.json`, and contributes the three Claude-targeted * helper skills. The projector reads * `/session-context.jsonl` (written by the managed Claude * hook) for `cwd` / `git_branch` and walks the local Claude JSONL * transcripts under `/.claude/projects` for native DAG identity. * - * Each attach/detach emits a `client.attach`/`client.detach` span - * tagged with `hyp_plugin`, `client_name`, `status`, and - * `restored=true|false`. + * `attach()` emits a `client.attach` span tagged with `hyp_plugin`, + * `client_name`, `status`, and `restored=true|false`. The reversing + * detach is the single core disk-driven undo (LLP 0045 §Part 3), not a + * per-adapter hook. * * @param {PluginActivationContext} ctx * @ref LLP 0016#knows-nothing-about-claude-or-codex [implements] — adapter requires the ai-gateway capability; registers client + upstream preset @@ -205,68 +206,6 @@ export async function activate(ctx) { { component: 'plugin.claude' } ) }, - /** @param {AiGatewayClientDetachContext} detachCtx */ - async detach(detachCtx) { - const homeDir = ctx.env.HOME ?? os.homedir() - const settingsPath = defaultSettingsPath(homeDir) - - return withSpan( - 'client.detach', - { - [Attr.PLUGIN]: PLUGIN_NAME, - [Attr.OPERATION]: 'client.detach', - client_name: CLIENT_NAME, - hyp_client: CLIENT_NAME, - dry_run: detachCtx.dryRun === true, - }, - async (span) => { - if (detachCtx.dryRun) { - span.setAttribute('status', 'ok') - span.setAttribute('restored', false) - writeDetachOutput(detachCtx, { - status: 'ok', - client: CLIENT_NAME, - dryRun: true, - settingsPath, - changed: false, - }) - return - } - try { - const result = await detach({ settingsPath }) - const restored = result.changed === true - span.setAttribute('status', 'ok') - span.setAttribute('restored', restored) - if (restored) { - logger.info('client.detach.write', { - hyp_plugin: PLUGIN_NAME, - hyp_client: CLIENT_NAME, - settings_path: settingsPath, - changed: true, - }) - } - writeDetachOutput(detachCtx, { - status: 'ok', - client: CLIENT_NAME, - dryRun: false, - settingsPath, - changed: restored, - removed: result.changed && result.removed !== undefined - ? result.removed - : undefined, - warning: result.changed && result.warning !== undefined - ? result.warning - : undefined, - }) - } catch (err) { - span.setAttribute('status', 'failed') - span.setAttribute('restored', false) - throw err - } - }, - { component: 'plugin.claude' } - ) - }, }) ctx.commands.register({ @@ -514,51 +453,3 @@ function writeAttachOutput(attachCtx, fields) { } } -/** - * Render detach output: machine-readable JSON when `json` is set, - * otherwise the human prose. Keeps the JSON shape stable so callers - * can grep it. - * - * @param {AiGatewayClientDetachContext} detachCtx - * @param {{ - * status: 'ok' | 'failed', - * client: string, - * dryRun: boolean, - * settingsPath: string, - * changed: boolean, - * removed?: string, - * warning?: string, - * }} fields - */ -function writeDetachOutput(detachCtx, fields) { - if (detachCtx.json) { - /** @type {Record} */ - const payload = { - status: fields.status, - action: 'detach', - client: fields.client, - dry_run: fields.dryRun, - settings_path: fields.settingsPath, - changed: fields.changed, - } - if (fields.removed !== undefined) payload.removed = fields.removed - if (fields.warning !== undefined) payload.warning = fields.warning - detachCtx.stdout.write(JSON.stringify(payload) + '\n') - return - } - if (fields.dryRun) { - detachCtx.stdout.write(`(dry-run) Would detach Claude Code from ${fields.settingsPath}\n`) - return - } - if (fields.changed) { - detachCtx.stdout.write(`✓ Claude Code reverted (${fields.settingsPath})\n`) - if (fields.removed !== undefined) { - detachCtx.stdout.write(` Removed ANTHROPIC_BASE_URL=${fields.removed}\n`) - } - if (fields.warning !== undefined) { - detachCtx.stdout.write(` warning: ${fields.warning}\n`) - } - } else { - detachCtx.stdout.write(`No HypAware marker found in ${fields.settingsPath}; nothing to do.\n`) - } -} diff --git a/hypaware-core/plugins-workspace/codex/src/index.js b/hypaware-core/plugins-workspace/codex/src/index.js index 773b390..fa34357 100644 --- a/hypaware-core/plugins-workspace/codex/src/index.js +++ b/hypaware-core/plugins-workspace/codex/src/index.js @@ -9,10 +9,10 @@ import { Attr, getLogger, withSpan } from '../../../../src/core/observability/in import { createCodexBackfillProvider } from './backfill.js' import { CODEX_CONFIG_SECTION, validateCodexConfig } from './config.js' import { createCodexExchangeProjector } from './exchange-projector.js' -import { attach, defaultConfigPath, detach } from './settings.js' +import { attach, defaultConfigPath } from './settings.js' /** - * @import { AiGatewayCapability, AiGatewayClientAttachContext, AiGatewayClientDetachContext, PluginActivationContext } from '../../../../collectivus-plugin-kernel-types.d.ts' + * @import { AiGatewayCapability, AiGatewayClientAttachContext, PluginActivationContext } from '../../../../collectivus-plugin-kernel-types.d.ts' */ const PLUGIN_NAME = '@hypaware/codex' @@ -38,13 +38,14 @@ export const configSection = { section: CODEX_CONFIG_SECTION, validate: validate * * Resolves the `hypaware.ai-gateway` capability, registers the * OpenAI-compatible upstream preset, wires Codex's config.toml - * attach/detach, and contributes the `hypaware-query` skill plus + * `attach()`, and contributes the `hypaware-query` skill plus * the AI report skills (adoption/improvement/security/spend) for * Codex installs. * - * Each attach/detach emits a `client.attach`/`client.detach` span - * tagged with `hyp_plugin`, `client_name`, `status`, and - * `restored=true|false`. + * `attach()` emits a `client.attach` span tagged with `hyp_plugin`, + * `client_name`, `status`, and `restored=true|false`. The reversing + * detach is the single core disk-driven undo (LLP 0045 §Part 3), not a + * per-adapter hook. * * @param {PluginActivationContext} ctx * @ref LLP 0016#knows-nothing-about-claude-or-codex [implements] — adapter requires the ai-gateway capability; registers client + upstream presets @@ -170,70 +171,6 @@ export async function activate(ctx) { { component: 'plugin.codex' } ) }, - /** @param {AiGatewayClientDetachContext} detachCtx */ - async detach(detachCtx) { - const configPath = resolveConfigPath(ctx) - - return withSpan( - 'client.detach', - { - [Attr.PLUGIN]: PLUGIN_NAME, - [Attr.OPERATION]: 'client.detach', - client_name: CLIENT_NAME, - hyp_client: CLIENT_NAME, - dry_run: detachCtx.dryRun === true, - }, - async (span) => { - if (detachCtx.dryRun) { - span.setAttribute('status', 'ok') - span.setAttribute('restored', false) - writeDetachOutput(detachCtx, { - status: 'ok', - client: CLIENT_NAME, - dryRun: true, - configPath, - changed: false, - }) - return - } - try { - const result = await detach({ configPath }) - const restored = result.changed === true - span.setAttribute('status', 'ok') - span.setAttribute('restored', restored) - if (restored) { - logger.info('client.detach.write', { - hyp_plugin: PLUGIN_NAME, - hyp_client: CLIENT_NAME, - config_path: configPath, - changed: true, - }) - } - writeDetachOutput(detachCtx, { - status: 'ok', - client: CLIENT_NAME, - dryRun: false, - configPath, - changed: restored, - removed: result.changed && result.removed !== undefined - ? result.removed - : undefined, - restoredValue: result.changed && result.restoredValue !== undefined - ? result.restoredValue - : undefined, - warning: result.changed && result.warning !== undefined - ? result.warning - : undefined, - }) - } catch (err) { - span.setAttribute('status', 'failed') - span.setAttribute('restored', false) - throw err - } - }, - { component: 'plugin.codex' } - ) - }, }) const skillsRoot = path.resolve(skillsRootDir(), 'skills') @@ -421,55 +358,3 @@ function writeAttachOutput(attachCtx, fields) { } } -/** - * Render detach output: JSON when the detach context sets `json`, - * otherwise the human prose. - * - * @param {AiGatewayClientDetachContext} detachCtx - * @param {{ - * status: 'ok' | 'failed', - * client: string, - * dryRun: boolean, - * configPath: string, - * changed: boolean, - * removed?: string, - * restoredValue?: string, - * warning?: string, - * }} fields - */ -function writeDetachOutput(detachCtx, fields) { - if (detachCtx.json) { - /** @type {Record} */ - const payload = { - status: fields.status, - action: 'detach', - client: fields.client, - dry_run: fields.dryRun, - config_path: fields.configPath, - changed: fields.changed, - } - if (fields.removed !== undefined) payload.removed = fields.removed - if (fields.restoredValue !== undefined) payload.restored_value = fields.restoredValue - if (fields.warning !== undefined) payload.warning = fields.warning - detachCtx.stdout.write(JSON.stringify(payload) + '\n') - return - } - if (fields.dryRun) { - detachCtx.stdout.write(`(dry-run) Would detach Codex from ${fields.configPath}\n`) - return - } - if (fields.changed) { - detachCtx.stdout.write(`✓ Codex reverted (${fields.configPath})\n`) - if (fields.removed !== undefined) { - detachCtx.stdout.write(` Removed base_url=${fields.removed}\n`) - } - if (fields.restoredValue !== undefined) { - detachCtx.stdout.write(` Restored model_provider=${fields.restoredValue}\n`) - } - if (fields.warning !== undefined) { - detachCtx.stdout.write(` warning: ${fields.warning}\n`) - } - } else { - detachCtx.stdout.write(`No HypAware marker found in ${fields.configPath}; nothing to do.\n`) - } -} diff --git a/hypaware-core/smoke/flows/claude_attach_detach.js b/hypaware-core/smoke/flows/claude_attach_detach.js index 25f9e5a..e768a22 100644 --- a/hypaware-core/smoke/flows/claude_attach_detach.js +++ b/hypaware-core/smoke/flows/claude_attach_detach.js @@ -220,9 +220,9 @@ export async function run({ harness, expect }) { (v) => typeof v === 'string' && v.length === 0 ) expect.that( - 'stdout: hyp detach reported the revert', + 'stdout: hyp detach reported the revert (core disk-driven undo, plugin-agnostic prose)', detachStdout.text(), - (v) => typeof v === 'string' && v.includes('Claude Code reverted') + (v) => typeof v === 'string' && v.includes('Detached claude') && v.includes(settingsPath) ) const afterDetach = await fs.readFile(settingsPath, 'utf8') diff --git a/src/core/cli/core_commands.js b/src/core/cli/core_commands.js index 948d2f5..ea42499 100644 --- a/src/core/cli/core_commands.js +++ b/src/core/cli/core_commands.js @@ -2,6 +2,7 @@ import fs from 'node:fs/promises' import { createRequire } from 'node:module' +import os from 'node:os' import path from 'node:path' import process from 'node:process' @@ -16,6 +17,8 @@ import { discoverInstalledPlugins } from '../runtime/installed.js' import { discoverBundledPlugins } from '../runtime/bundled.js' import { isWithinDir } from '../runtime/contribution_names.js' import { buildPluginCatalog } from '../plugin_catalog.js' +import { detachClientFromDisk } from '../config/client_detach_disk.js' +import { resolveClientSettingsPath } from '../daemon/client_settings_path.js' import { collectHypAwareStatus } from '../daemon/status.js' import { renderSchema, schemaForDataset } from '../query/schema.js' import { createMcpServer } from '../mcp/server.js' @@ -3189,13 +3192,15 @@ async function runAttach(argv, ctx) { /** * `hyp detach [client] [--client ]` * - * Resolves the gateway capability, looks up the named client, and - * dispatches to its `detach()`. `detach()` is invoked with the - * adapter's config slice (currently empty until per-adapter config - * lands) plus stdout/stderr. + * Reverses a client's attach. Unlike `attach`, detach does **not** + * dispatch to a per-adapter hook: it routes through the single core, + * disk-driven undo (`detachClientFromDisk`), resolved per client via its + * `clientDescriptor`. That one undo is shared with the daemon + * reconciler's `reverse()`, so the two can never drift. * * @param {string[]} argv * @param {CommandRunContext} ctx + * @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [implements] — manual detach routes through the one core undo via the clientDescriptor, not a per-adapter detach() */ async function runDetach(argv, ctx) { return runClientLifecycle('detach', argv, ctx) @@ -3261,6 +3266,11 @@ async function runClientLifecycle(action, argv, ctx) { return 1 } + // Detach is the single core disk-driven undo, resolved per client via + // its static descriptor (owning plugin + attach_probe) — not a + // per-adapter detach() hook (LLP 0045 §Part 3). Build the map once. + const clientDescriptors = action === 'detach' ? await buildClientDescriptorMap() : undefined + let exitCode = 0 for (const name of clientNames) { const client = gateway.getClient(name) @@ -3300,12 +3310,12 @@ async function runClientLifecycle(action, argv, ctx) { json: parsed.json, }) } else { - await client.detach({ - config: {}, - stdout: ctx.stdout, - stderr: ctx.stderr, + await detachClientViaCore({ + name, + descriptor: clientDescriptors?.get(name), dryRun: parsed.dryRun, json: parsed.json, + ctx, }) } } catch (err) { @@ -3317,6 +3327,137 @@ async function runClientLifecycle(action, argv, ctx) { return exitCode } +/** + * Reverse a client's attach from disk — the single core undo + * (`detachClientFromDisk`). The manual `hyp detach` command and the + * daemon reconciler's `reverse()` both route through this one + * implementation, resolved per client via its `descriptor` (owning + * plugin + `attach_probe`), so there is no per-adapter detach for the + * one undo to drift from. Emits a `client.detach` span and the same + * `done`/`no-op` output shape callers grep. + * + * @param {{ + * name: string, + * descriptor: ClientDescriptor | undefined, + * dryRun: boolean, + * json: boolean, + * ctx: CommandRunContext, + * }} args + * @returns {Promise} + * @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [implements] — manual detach is the disk-driven core undo, resolved via the clientDescriptor; one undo, shared with the reconciler reverse() + */ +async function detachClientViaCore({ name, descriptor, dryRun, json, ctx }) { + if (!descriptor) { + throw new Error(`no client descriptor for '${name}'; cannot reverse its attach from disk`) + } + const homeDir = ctx.env.HOME ?? os.homedir() + return withSpan( + 'client.detach', + { + [Attr.PLUGIN]: descriptor.plugin, + [Attr.OPERATION]: 'client.detach', + client_name: name, + hyp_client: name, + dry_run: dryRun === true, + }, + async (span) => { + if (dryRun) { + span.setAttribute('status', 'ok') + span.setAttribute('restored', false) + const settingsPath = descriptor.attachProbe + ? resolveClientSettingsPath(name, descriptor.attachProbe.settings_file, ctx.env, homeDir) + : undefined + if (json) { + ctx.stdout.write( + JSON.stringify({ + status: 'ok', + action: 'detach', + client: name, + dry_run: true, + ...(settingsPath !== undefined ? { settings_path: settingsPath } : {}), + changed: false, + }) + '\n' + ) + } else { + ctx.stdout.write( + `(dry-run) Would detach ${name}${settingsPath !== undefined ? ` from ${settingsPath}` : ''}\n` + ) + } + return + } + try { + const result = await detachClientFromDisk({ descriptor, homeDir, env: ctx.env }) + const restored = result.changed === true + span.setAttribute('status', 'ok') + span.setAttribute('restored', restored) + if (restored) { + getLogger('cmd-detach').info('client.detach.write', { + hyp_client: name, + hyp_plugin: descriptor.plugin, + settings_path: result.settingsPath, + changed: true, + }) + } + writeCoreDetachOutput({ ctx, name, json, result }) + } catch (err) { + span.setAttribute('status', 'failed') + span.setAttribute('restored', false) + throw err + } + }, + { component: 'cmd-detach' } + ) +} + +/** + * Render the core detach output: machine-readable JSON when `json` is + * set, otherwise human prose. The shape mirrors the retired adapter + * output (`status`/`action`/`client`/`settings_path`/`changed`) so + * callers that grepped it keep working. + * + * @param {{ + * ctx: CommandRunContext, + * name: string, + * json: boolean, + * result: { + * changed: boolean, + * settingsPath?: string, + * removed?: string, + * restoredValue?: string, + * warning?: string, + * }, + * }} args + */ +function writeCoreDetachOutput({ ctx, name, json, result }) { + const settingsPath = result.settingsPath + if (json) { + /** @type {Record} */ + const payload = { + status: 'ok', + action: 'detach', + client: name, + dry_run: false, + changed: result.changed === true, + } + if (settingsPath !== undefined) payload.settings_path = settingsPath + if (result.removed !== undefined) payload.removed = result.removed + if (result.restoredValue !== undefined) payload.restored_value = result.restoredValue + if (result.warning !== undefined) payload.warning = result.warning + ctx.stdout.write(JSON.stringify(payload) + '\n') + return + } + if (result.changed === true) { + ctx.stdout.write(`✓ Detached ${name}${settingsPath !== undefined ? ` (${settingsPath})` : ''}\n`) + if (result.removed !== undefined) ctx.stdout.write(` Removed ${result.removed}\n`) + if (result.restoredValue !== undefined) ctx.stdout.write(` Restored ${result.restoredValue}\n`) + if (result.warning !== undefined) ctx.stdout.write(` warning: ${result.warning}\n`) + } else { + ctx.stdout.write( + `No HypAware marker found${settingsPath !== undefined ? ` in ${settingsPath}` : ''}; nothing to do.\n` + ) + } +} + /** * Resolve the gateway endpoint from the active config when the gateway * source is not live in this process yet. This is the normal shape for diff --git a/test/core/command-dispatch.test.js b/test/core/command-dispatch.test.js index 5a42540..f4e0849 100644 --- a/test/core/command-dispatch.test.js +++ b/test/core/command-dispatch.test.js @@ -226,24 +226,31 @@ test('attach accepts a positional client name', async () => { ]) }) -test('unattach alias accepts a positional client name', async () => { +test('unattach alias routes a positional client through the core disk undo', async () => { const { registry, kernel, calls } = fakeClientKernel() const stdout = makeBuf() const stderr = makeBuf() + // Isolate HOME/CODEX_HOME so the disk-driven detach targets a temp tree + // (no marker present → a clean no-op) rather than the developer's files. + const home = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-unattach-')) const code = await dispatch(['unattach', 'claude', '--json'], { stdout, stderr, registry, kernel, - env: { ...process.env, HYP_HOME: await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-unattach-')) }, + env: { ...process.env, HOME: home, CODEX_HOME: home, HYP_HOME: home }, }) assert.equal(code, 0) assert.equal(stderr.text(), '') - assert.deepEqual(calls, [ - { action: 'detach', client: 'claude', dryRun: false, json: true }, - ]) + // Detach no longer dispatches to a per-adapter hook — it is the single + // core disk-driven undo (LLP 0045 §Part 3), so the fake client's + // detach() is never called. + assert.deepEqual(calls, []) + const out = JSON.parse(stdout.text().trim()) + assert.equal(out.action, 'detach') + assert.equal(out.client, 'claude') }) test('attach rejects conflicting positional and flag client names', async () => { diff --git a/test/core/integration.test.js b/test/core/integration.test.js index 29ea2bb..293c6e8 100644 --- a/test/core/integration.test.js +++ b/test/core/integration.test.js @@ -100,10 +100,14 @@ test('attach returns the parsed structured result', async () => { assert.deepEqual(calls, [{ action: 'attach', client: 'claude', json: true }]) }) -test('detach returns the parsed structured result', async () => { +test('detach returns the parsed structured result (core disk undo)', async () => { const { registry, kernel, calls } = fakeClientKernel() + // Isolate HOME/CODEX_HOME so the disk-driven detach targets a temp tree + // (no marker present → a clean no-op) rather than the developer's files. + const home = await freshHome() const result = await detach('codex', { - hypHome: await freshHome(), + hypHome: home, + env: { ...process.env, HOME: home, CODEX_HOME: home }, // @ts-expect-error test-only kernel injection registry, kernel, @@ -111,7 +115,9 @@ test('detach returns the parsed structured result', async () => { assert.equal(result.status, 'ok') assert.equal(result.action, 'detach') assert.equal(result.client, 'codex') - assert.deepEqual(calls, [{ action: 'detach', client: 'codex', json: true }]) + // Detach is the single core disk-driven undo (LLP 0045 §Part 3), not a + // per-adapter hook — the fake client's detach() is never dispatched. + assert.deepEqual(calls, []) }) test('attach throws HypAwareCommandError for an unknown client', async () => { diff --git a/test/plugins/ai-gateway-api.test.js b/test/plugins/ai-gateway-api.test.js index c120723..47d96ab 100644 --- a/test/plugins/ai-gateway-api.test.js +++ b/test/plugins/ai-gateway-api.test.js @@ -112,37 +112,38 @@ test('registerUpstreamPreset replaces an existing entry with the same name', () assert.equal(state.presets.get('echo')?.base_url, 'http://b') }) -test('registerClient validates name, defaultUpstream, and attach/detach', () => { +test('registerClient validates name, defaultUpstream, and attach', () => { const state = createGatewayState() const api = createAiGatewayApi(state) + // An adapter owns only attach(); the reversing detach is the single + // core disk-driven undo (LLP 0045 §Part 3), so detach is not required. const ok = { name: 'codex', defaultUpstream: 'openai', attach: async () => {}, - detach: async () => {}, } api.registerClient(ok) assert.equal(state.clients.get('codex')?.defaultUpstream, 'openai') assert.throws( - () => api.registerClient(/** @type {any} */ ({ defaultUpstream: 'u', attach() {}, detach() {} })), + () => api.registerClient(/** @type {any} */ ({ defaultUpstream: 'u', attach() {} })), /name is required/ ) assert.throws( - () => api.registerClient(/** @type {any} */ ({ name: 'x', attach() {}, detach() {} })), + () => api.registerClient(/** @type {any} */ ({ name: 'x', attach() {} })), /defaultUpstream is required/ ) assert.throws( - () => api.registerClient(/** @type {any} */ ({ name: 'x', defaultUpstream: 'u', attach() {} })), - /attach\(\)\/detach\(\) are required/ + () => api.registerClient(/** @type {any} */ ({ name: 'x', defaultUpstream: 'u' })), + /attach\(\) is required/ ) }) test('getClient and listClients expose the registered clients', () => { const state = createGatewayState() const api = createAiGatewayApi(state) - const claude = { name: 'claude', defaultUpstream: 'anthropic', attach: async () => {}, detach: async () => {} } - const codex = { name: 'codex', defaultUpstream: 'openai', attach: async () => {}, detach: async () => {} } + const claude = { name: 'claude', defaultUpstream: 'anthropic', attach: async () => {} } + const codex = { name: 'codex', defaultUpstream: 'openai', attach: async () => {} } api.registerClient(claude) api.registerClient(codex) assert.equal(api.getClient('claude'), claude) From 58b1feb4843428d31a5bf3a7ea17528e0a4b3cc0 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 15:23:55 -0700 Subject: [PATCH 10/15] =?UTF-8?q?Client=20attach:=20daemon=20wiring=20?= =?UTF-8?q?=E2=80=94=20resolve=20client=20seam=20+=20register=20[attachHan?= =?UTF-8?q?dler,=20backfillHandler]=20(LLP=200045=20T7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the attach handler into the daemon's client-action reconciler (LLP 0045 §Part 1 / Module-seam-breakdown item 7): - runtime.js: resolve the client-action seam from boot — clientDescriptors from the plugin catalog (now surfaced on BootKernelResult), and clients/endpoint from boot.runtime.capabilities when the ai-gateway capability is present, via localEndpoint() with the configuredGatewayEndpoint fallback. Thread all three onto each reconcile() input and register [attachHandler, backfillHandler] — attach first (exported DEFAULT_ACTION_HANDLERS so the order is a testable invariant). - action_reconciler.js: thread the new context fields (clientDescriptors / clients / endpoint) onto the ActionContext so a client handler can read them. - boot.js / runtime types: surface the boot catalog's clientDescriptors on BootKernelResult. - gateway_endpoint.js: extract configuredGatewayEndpoint/endpointFromListen into a shared module (was duplicated in core_commands.js and walkthrough.js; the daemon is the third consumer). Tests: daemon-reconcile.test.js gains the handler-order invariant and a gateway-enabled boot asserting clientDescriptors/clients/endpoint are threaded (and undefined on a non-gateway boot). New hermetic smoke client_attach_on_join drives join -> auto-attach -> no-re-attach -> config-drop reverse end to end. typecheck, lint, full test suite (1522 pass), and the new + existing attach/join smokes are green. (Pre-existing unrelated red: walkthrough_to_first_query, also red on the untouched integration branch.) Co-Authored-By: Claude Opus 4.8 (1M context) Task-Id: T7 --- .../smoke/flows/client_attach_on_join.js | 483 ++++++++++++++++++ src/core/cli/core_commands.js | 40 +- src/core/cli/walkthrough.js | 39 +- src/core/config/action_reconciler.js | 17 +- src/core/config/gateway_endpoint.js | 54 ++ src/core/daemon/runtime.js | 109 +++- src/core/runtime/boot.js | 2 + src/core/runtime/types.d.ts | 10 + test/core/daemon-reconcile.test.js | 95 +++- 9 files changed, 763 insertions(+), 86 deletions(-) create mode 100644 hypaware-core/smoke/flows/client_attach_on_join.js create mode 100644 src/core/config/gateway_endpoint.js diff --git a/hypaware-core/smoke/flows/client_attach_on_join.js b/hypaware-core/smoke/flows/client_attach_on_join.js new file mode 100644 index 0000000..e452813 --- /dev/null +++ b/hypaware-core/smoke/flows/client_attach_on_join.js @@ -0,0 +1,483 @@ +// @ts-check + +import fs from 'node:fs/promises' +import http from 'node:http' +import path from 'node:path' +import process from 'node:process' + +import { installObservability } from '../../../src/core/observability/index.js' +import { defaultConfigPath } from '../../../src/core/config/schema.js' +import { readConfigControlStatus } from '../../../src/core/config/apply.js' +import { readClientActionStatus } from '../../../src/core/config/action_reconciler.js' +import { DAEMON_RESTART_EXIT_CODE, runDaemon } from '../../../src/core/daemon/runtime.js' +import { dispatch } from '../../../src/core/cli/dispatch.js' + +/** + * @import { AddressInfo } from 'node:net' + */ + +/** + * End-to-end auto-attach / reverse smoke (LLP 0044 / LLP 0045 §Part 5, T7). + * + * Drives the headline client-attach lifecycle against a stub central server, + * proving the daemon wiring (T7) carries the attach handler end to end: + * + * 1. join → seed boot → pull rev-1 (central + ai-gateway + claude) → apply → + * staged restart. + * 2. relaunch on rev-1 → first poll clears probation → the confirmation edge + * schedules a reconcile pass → **claude auto-attaches**: the `_hypaware` + * marker + the gateway `ANTHROPIC_BASE_URL` land in the client settings, + * and the `attach.claude` client-action marker reads `done`. + * 3. a second confirmed boot pass (a fresh relaunch on the same rev-1) is a + * **no-op** — the `done` marker short-circuits, so the attach is not + * re-applied (the marker timestamp is unchanged). + * 4. the server drops `@hypaware/claude` (rev-2) → apply → staged restart → + * relaunch without the adapter → the reconcile **reverse gap** runs the + * disk-driven undo: the marker is removed and the client settings are + * restored to their pre-attach state — the Part 5 config-drop trigger, + * exercised post-restart with the adapter already unloaded. + * + * The daemon runs in-process; the smoke plays the foreground invoker, relaunching + * `runDaemon` whenever `handle.done` resolves with the restart exit code. + * + * @param {{ harness: any, expect: any }} args + * @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [tests] — the daemon threads clientDescriptors/clients/endpoint onto the reconcile context; a confirm-edge pass reaches the attach handler + * @ref LLP 0045#part-5--reverse-triggers-config-drop-not-hyp-leave [tests] — a central config drop reverses the attach post-restart via the disk-driven undo + * @ref LLP 0044#consent--join-implies-consent-default-on [tests] — a joined host confirming a config that names @hypaware/claude auto-attaches (default-on) + */ +export async function run({ harness, expect }) { + const obs = installObservability() + if (!obs.tracer.provider) { + throw new Error( + 'client_attach_on_join: tracer provider not installed — expected HYP_DEV_TELEMETRY=1' + ) + } + + const fakeHome = path.join(harness.tmpDir, 'home') + await fs.mkdir(path.join(fakeHome, '.claude'), { recursive: true }) + const claudeSettingsPath = path.join(fakeHome, '.claude', 'settings.json') + // Seed unrelated user content so the round-trip can prove attach/reverse + // preserves it byte-for-byte. + const seedClaudeBody = JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-seed' } }, null, 2) + '\n' + await fs.writeFile(claudeSettingsPath, seedClaudeBody, 'utf8') + + const previousHome = process.env.HOME + const previousClaudeHome = process.env.CLAUDE_HOME + process.env.HOME = fakeHome + delete process.env.CLAUDE_HOME + + process.env.HYP_HOME = harness.hypHome + delete process.env.HYP_CONFIG + const localConfigPath = defaultConfigPath(harness.hypHome) + const stateRoot = path.join(harness.hypHome, 'hypaware') + + const server = await startStubCentralServer() + try { + // rev-1: a joined fleet config that enables the gateway + the claude client + // adapter. Confirming it must auto-attach claude. + server.setConfig(rev1Config(server.baseUrl), 'rev-1') + + // An empty local layer so `join` has something to leave untouched. + await fs.writeFile(localConfigPath, JSON.stringify({ version: 2, plugins: [] }, null, 2) + '\n') + + // ----- smoke_step: join (seed the central layer) ----- + const joinOut = makeBuf() + const joinErr = makeBuf() + const joinCode = await dispatch( + ['join', server.baseUrl, 'policy-token-attach', '--no-daemon'], + { stdout: joinOut, stderr: joinErr, env: { ...process.env, HYP_HOME: harness.hypHome } } + ) + expect.that(`join: exits 0 (stderr: ${joinErr.text()})`, joinCode, (v) => v === 0) + + // ----- smoke_step: seed_boot (bootstrap → pull → apply → restart) ----- + const firstExit = await bootOnceForRestart(harness) + expect.that( + `seed boot: daemon exited with the restart code (got ${firstExit})`, + firstExit, + (v) => v === DAEMON_RESTART_EXIT_CODE + ) + + // ----- smoke_step: auto_attach (relaunch rev-1 → confirm → attach) ----- + const attachHandle = await runDaemonHandle(harness) + try { + await waitFor( + () => readConfigControlStatus({ stateRoot }).probation === null, + 15_000, + 'probation did not clear within 15s of the rev-1 relaunch' + ) + // The confirmation edge schedules the reconcile pass that attaches claude. + await waitFor( + () => attachMarker(stateRoot)?.status === 'done', + 15_000, + 'the attach.claude marker did not reach done after the confirmation edge' + ) + + const attached = JSON.parse(await fs.readFile(claudeSettingsPath, 'utf8')) + expect.that( + 'auto-attach: the _hypaware marker was written to the client settings', + attached?._hypaware, + (v) => v !== null && typeof v === 'object' && typeof v.port === 'number' + ) + expect.that( + 'auto-attach: env.ANTHROPIC_BASE_URL points at the local gateway', + attached?.env?.ANTHROPIC_BASE_URL, + (v) => typeof v === 'string' && /^http:\/\/127\.0\.0\.1:\d+$/.test(v) + ) + expect.that( + 'auto-attach: the unrelated seed key (ANTHROPIC_API_KEY) survived attach', + attached?.env?.ANTHROPIC_API_KEY, + (v) => v === 'sk-seed' + ) + const doneMarker = attachMarker(stateRoot) + expect.that( + 'auto-attach: the client-action marker reads done for the claude request key', + doneMarker?.request_key, + (v) => v === 'claude' + ) + } finally { + await attachHandle.stop() + await attachHandle.done + } + + // Snapshot the post-attach state for the idempotency assertion below. + const attachedAt = attachMarker(stateRoot)?.at + const attachedBody = await fs.readFile(claudeSettingsPath, 'utf8') + + // ----- smoke_step: no_reattach (a second confirmed boot pass is a no-op) ----- + // A fresh relaunch on the *same* rev-1 runs the after-activation + // already-confirmed pass (probation is cleared), so desired() names claude + // again — but the `done` marker short-circuits, so nothing is re-applied. + const steadyHandle = await runDaemonHandle(harness) + try { + await waitFor( + () => readConfigControlStatus({ stateRoot }).probation === null, + 15_000, + 'probation was unexpectedly re-armed on the steady relaunch' + ) + // Give the boot-already-confirmed pass time to run (and prove it does not + // re-attach): the marker timestamp must be identical. + await sleep(500) + expect.that( + 'no re-attach: the attach.claude marker timestamp is unchanged (done short-circuits)', + attachMarker(stateRoot)?.at, + (v) => v === attachedAt + ) + expect.that( + 'no re-attach: the client settings are byte-for-byte unchanged', + await fs.readFile(claudeSettingsPath, 'utf8'), + (v) => v === attachedBody + ) + + // ----- smoke_step: drop_claude (serve rev-2 → apply → restart) ----- + // rev-2 drops @hypaware/claude fleet-wide; the running daemon's next poll + // applies it and requests a staged restart. + server.setConfig(rev2Config(server.baseUrl), 'rev-2') + const dropExit = await withTimeout( + steadyHandle.done, + 30_000, + 'the rev-2 drop did not request a staged restart within 30s' + ) + expect.that( + `drop: daemon exited with the restart code (got ${dropExit})`, + dropExit, + (v) => v === DAEMON_RESTART_EXIT_CODE + ) + } finally { + // `steadyHandle.done` already resolved (restart) — stop() is idempotent. + await steadyHandle.stop() + } + + // ----- smoke_step: reverse (relaunch rev-2 → reverse gap → restore) ----- + const reverseHandle = await runDaemonHandle(harness) + try { + await waitFor( + () => readConfigControlStatus({ stateRoot }).probation === null, + 15_000, + 'probation did not clear within 15s of the rev-2 relaunch' + ) + // The reverse gap removes the marker once the disk-driven undo succeeds. + await waitFor( + () => attachMarker(stateRoot) === undefined, + 15_000, + 'the attach.claude marker was not removed by the reverse gap' + ) + + const restored = await fs.readFile(claudeSettingsPath, 'utf8') + expect.that( + 'reverse: the _hypaware marker was stripped from the client settings', + JSON.parse(restored)?._hypaware, + (v) => v === undefined + ) + expect.that( + 'reverse: the managed ANTHROPIC_BASE_URL was removed (no prior to restore)', + JSON.parse(restored)?.env?.ANTHROPIC_BASE_URL, + (v) => v === undefined + ) + expect.that( + 'reverse: the unrelated seed key (ANTHROPIC_API_KEY) survived the round-trip', + JSON.parse(restored)?.env?.ANTHROPIC_API_KEY, + (v) => v === 'sk-seed' + ) + } finally { + await reverseHandle.stop() + await reverseHandle.done + } + } finally { + await server.close() + if (previousHome === undefined) delete process.env.HOME + else process.env.HOME = previousHome + if (previousClaudeHome === undefined) delete process.env.CLAUDE_HOME + else process.env.CLAUDE_HOME = previousClaudeHome + } + + await obs.shutdown() + + // ----- smoke_step: telemetry ----- + const logs = await expect.logs() + expect.that( + 'logs: the reconciler recorded a done attach for the claude request key', + logs.some((/** @type {any} */ l) => + l.body === 'client_action.done' && + l.attributes?.kind === 'attach' && + l.attributes?.request_key === 'claude' + ), + (v) => v === true + ) + expect.that( + 'logs: the reconciler recorded a reversed attach for the claude request key', + logs.some((/** @type {any} */ l) => + l.body === 'client_action.reversed' && + l.attributes?.kind === 'attach' && + l.attributes?.request_key === 'claude' + ), + (v) => v === true + ) +} + +/* ---------- served revisions ---------- */ + +/** @param {string} baseUrl */ +function rev1Config(baseUrl) { + return { + version: 2, + plugins: [ + { name: '@hypaware/central' }, + { + name: '@hypaware/ai-gateway', + config: { + listen: '127.0.0.1:0', + upstreams: [ + { name: 'anthropic', base_url: 'https://api.anthropic.com', path_prefix: '/' }, + ], + }, + }, + { name: '@hypaware/claude' }, + ], + sinks: centralSink(baseUrl), + query: { cache: { retention: { default_days: 30 } } }, + } +} + +/** rev-2 is rev-1 minus the claude client plugin — the fleet-drop trigger. @param {string} baseUrl */ +function rev2Config(baseUrl) { + return { + version: 2, + plugins: [ + { name: '@hypaware/central' }, + { + name: '@hypaware/ai-gateway', + config: { + listen: '127.0.0.1:0', + upstreams: [ + { name: 'anthropic', base_url: 'https://api.anthropic.com', path_prefix: '/' }, + ], + }, + }, + ], + sinks: centralSink(baseUrl), + query: { cache: { retention: { default_days: 30 } } }, + } +} + +/** @param {string} baseUrl */ +function centralSink(baseUrl) { + return { + central: { + plugin: '@hypaware/central', + config: { + url: baseUrl, + identity: {}, + schedule: '0 * * * *', + poll_interval_seconds: 5, + }, + }, + } +} + +/* ---------- daemon lifecycle helpers ---------- */ + +/** + * Boot the daemon once and await its `done` — used for a boot that is expected + * to apply a served revision and request a staged restart. + * @param {{ hypHome: string, devRunId: string }} harness + * @returns {Promise} + */ +async function bootOnceForRestart(harness) { + const handle = await runDaemonHandle(harness) + return withTimeout( + handle.done, + 30_000, + 'the boot did not request a staged restart within 30s' + ) +} + +/** + * @param {{ hypHome: string, devRunId: string }} harness + */ +async function runDaemonHandle(harness) { + return runDaemon({ + hypHome: harness.hypHome, + env: process.env, + runId: harness.devRunId, + tickIntervalMs: 0, + installSignalHandlers: false, + }) +} + +/** + * Read the `attach.claude` client-action marker, or `undefined` when absent. + * @param {string} stateRoot + * @returns {{ status?: string, request_key?: string, at?: string } | undefined} + */ +function attachMarker(stateRoot) { + const byKind = readClientActionStatus({ stateRoot }).byKind + const attach = /** @type {Record | undefined} */ (byKind.attach) + return attach?.claude +} + +/* ---------- stub central server (mirrors join_flow_remote_config) ---------- */ + +async function startStubCentralServer() { + /** @type {Array<{ method: string, path: string, ifNoneMatch?: string, responseStatus: number }>} */ + const requests = [] + /** @type {unknown} */ + let configDoc = null + /** @type {string} */ + let configEtag = '' + + const jwt = buildFakeJwt('gateway-attach-1') + const expiresAt = Math.floor(Date.now() / 1000) + 30 * 24 * 60 * 60 + + const server = http.createServer((req, res) => { + const url = new URL(req.url ?? '/', 'http://localhost') + /** @param {number} status @param {Record} headers @param {string} [body] */ + function reply(status, headers, body) { + requests.push({ + method: req.method ?? '', + path: url.pathname, + ...(req.headers['if-none-match'] ? { ifNoneMatch: String(req.headers['if-none-match']) } : {}), + responseStatus: status, + }) + res.writeHead(status, headers) + res.end(body ?? '') + } + + if (req.method === 'POST' && (url.pathname === '/v1/identity/bootstrap' || url.pathname === '/v1/identity/refresh')) { + reply(200, { 'content-type': 'application/json' }, JSON.stringify({ jwt, expires_at: expiresAt })) + return + } + if (req.method === 'GET' && url.pathname === '/v1/config') { + if (!configDoc) { + reply(404, { 'content-type': 'application/json' }, JSON.stringify({ error: 'no_config' })) + return + } + if (req.headers['if-none-match'] === configEtag) { + reply(304, { etag: configEtag }) + return + } + reply(200, { 'content-type': 'application/json', etag: configEtag }, JSON.stringify(configDoc)) + return + } + if (req.method === 'POST' && url.pathname.startsWith('/v1/ingest/')) { + reply(202, {}) + return + } + reply(404, { 'content-type': 'application/json' }, JSON.stringify({ error: 'not_found' })) + }) + + await new Promise((resolve) => server.listen(0, '127.0.0.1', () => resolve(undefined))) + const address = /** @type {AddressInfo} */ (server.address()) + + return { + baseUrl: `http://127.0.0.1:${address.port}`, + requests, + /** @param {unknown} doc @param {string} etag */ + setConfig(doc, etag) { + configDoc = doc + configEtag = etag + }, + close() { + return new Promise((resolve) => server.close(() => resolve(undefined))) + }, + } +} + +/** @param {string} sub */ +function buildFakeJwt(sub) { + /** @param {object} obj */ + const b64 = (obj) => Buffer.from(JSON.stringify(obj)).toString('base64url') + return `${b64({ alg: 'none', typ: 'JWT' })}.${b64({ sub })}.smoke` +} + +/* ---------- generic helpers ---------- */ + +function makeBuf() { + let value = '' + return { + /** @param {string} chunk */ + write(chunk) { + value += String(chunk) + return true + }, + text() { + return value + }, + } +} + +/** @param {number} ms */ +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +/** + * @template T + * @param {Promise} promise + * @param {number} ms + * @param {string} message + * @returns {Promise} + */ +function withTimeout(promise, ms, message) { + /** @type {NodeJS.Timeout} */ + let timer + return Promise.race([ + promise.finally(() => clearTimeout(timer)), + new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error(`client_attach_on_join: ${message}`)), ms) + }), + ]) +} + +/** + * @param {() => boolean} predicate + * @param {number} ms + * @param {string} message + */ +async function waitFor(predicate, ms, message) { + const deadline = Date.now() + ms + while (Date.now() < deadline) { + if (predicate()) return + await sleep(50) + } + throw new Error(`client_attach_on_join: ${message}`) +} diff --git a/src/core/cli/core_commands.js b/src/core/cli/core_commands.js index ea42499..963b996 100644 --- a/src/core/cli/core_commands.js +++ b/src/core/cli/core_commands.js @@ -18,6 +18,7 @@ import { discoverBundledPlugins } from '../runtime/bundled.js' import { isWithinDir } from '../runtime/contribution_names.js' import { buildPluginCatalog } from '../plugin_catalog.js' import { detachClientFromDisk } from '../config/client_detach_disk.js' +import { configuredGatewayEndpoint } from '../config/gateway_endpoint.js' import { resolveClientSettingsPath } from '../daemon/client_settings_path.js' import { collectHypAwareStatus } from '../daemon/status.js' import { renderSchema, schemaForDataset } from '../query/schema.js' @@ -3458,45 +3459,6 @@ function writeCoreDetachOutput({ ctx, name, json, result }) { } } -/** - * Resolve the gateway endpoint from the active config when the gateway - * source is not live in this process yet. This is the normal shape for - * commands like `hyp attach`, which only need to write client settings - * to the same fixed port the daemon will bind later. - * - * @param {HypAwareV2Config} config - * @returns {string | undefined} - */ -function configuredGatewayEndpoint(config) { - const entry = config.plugins?.find((p) => p.name === '@hypaware/ai-gateway') - const cfg = entry?.config - if (!cfg || typeof cfg !== 'object' || Array.isArray(cfg)) return undefined - const listen = /** @type {Record} */ (cfg).listen - if (typeof listen !== 'string') return undefined - return endpointFromListen(listen) -} - -/** - * @param {string} listen - * @returns {string | undefined} - */ -function endpointFromListen(listen) { - const idx = listen.lastIndexOf(':') - if (idx === -1) return undefined - const rawHost = listen.slice(0, idx) - const rawPort = listen.slice(idx + 1) - const port = Number.parseInt(rawPort, 10) - if (!Number.isInteger(port) || port < 1 || port > 65535 || String(port) !== rawPort) { - return undefined - } - const host = rawHost.startsWith('[') && rawHost.endsWith(']') - ? rawHost.slice(1, -1) - : rawHost - if (host.length === 0) return undefined - const formattedHost = host.includes(':') && !host.startsWith('[') ? `[${host}]` : host - return `http://${formattedHost}:${port}` -} - /** * Parse an optional positional client name plus `--client `, * `--yes` / `-y`, `--dry-run`, and `--json` from argv. diff --git a/src/core/cli/walkthrough.js b/src/core/cli/walkthrough.js index 9ae56b3..2732bc5 100644 --- a/src/core/cli/walkthrough.js +++ b/src/core/cli/walkthrough.js @@ -6,6 +6,7 @@ import readline from 'node:readline/promises' import { Attr, getLogger, withSpan } from '../observability/index.js' import { defaultConfigPath, prepareLocalConfigWrite } from '../config/schema.js' +import { configuredGatewayEndpoint } from '../config/gateway_endpoint.js' import { readObservabilityEnv } from '../observability/env.js' import { discoverBundledPlugins } from '../runtime/bundled.js' import { isWithinDir } from '../runtime/contribution_names.js' @@ -1426,44 +1427,6 @@ async function stopFinaleStartedSources(sources) { } } -/** - * Resolve the gateway endpoint from the just-written config. Init - * attaches clients before the daemon's gateway source is live in this - * process, so `localEndpoint()` cannot be the only source of truth. - * - * @param {HypAwareV2Config} config - * @returns {string | undefined} - */ -function configuredGatewayEndpoint(config) { - const entry = config.plugins?.find((p) => p.name === '@hypaware/ai-gateway') - const cfg = entry?.config - if (!cfg || typeof cfg !== 'object' || Array.isArray(cfg)) return undefined - const listen = /** @type {Record} */ (cfg).listen - if (typeof listen !== 'string') return undefined - return endpointFromListen(listen) -} - -/** - * @param {string} listen - * @returns {string | undefined} - */ -function endpointFromListen(listen) { - const idx = listen.lastIndexOf(':') - if (idx === -1) return undefined - const rawHost = listen.slice(0, idx) - const rawPort = listen.slice(idx + 1) - const port = Number.parseInt(rawPort, 10) - if (!Number.isInteger(port) || port < 1 || port > 65535 || String(port) !== rawPort) { - return undefined - } - const host = rawHost.startsWith('[') && rawHost.endsWith(']') - ? rawHost.slice(1, -1) - : rawHost - if (host.length === 0) return undefined - const formattedHost = host.includes(':') && !host.startsWith('[') ? `[${host}]` : host - return `http://${formattedHost}:${port}` -} - /** * @returns {Promise>} */ diff --git a/src/core/config/action_reconciler.js b/src/core/config/action_reconciler.js index 8ccf16d..1120c61 100644 --- a/src/core/config/action_reconciler.js +++ b/src/core/config/action_reconciler.js @@ -81,8 +81,23 @@ export function createActionReconciler(opts) { * @ref LLP 0041#the-reconciler-component [implements] — reconcile() is level-triggered: diff desired() against the marker, act only on the gap; a done marker short-circuits */ async function reconcile(input) { + // Thread the daemon-resolved client seam (LLP 0045 §Part 1) onto the + // context unchanged — the reconciler core stays ignorant of what they mean + // ("knows nothing about Claude vs Codex"); only a client handler + // (`action_attach`) reads `clientDescriptors`/`clients`/`endpoint`. Absent + // on a plain CLI boot, so any client handler stays inert. + // @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — clientDescriptors/clients/endpoint live on the context, not a handler closure /** @type {ActionContext} */ - const ctx = { config: input.config, backfills: input.backfills, env: input.env, now, log } + const ctx = { + config: input.config, + backfills: input.backfills, + env: input.env, + clientDescriptors: input.clientDescriptors, + clients: input.clients, + endpoint: input.endpoint, + now, + log, + } const store = readStore() /** @type {ReconcileActionResult[]} */ const results = [] diff --git a/src/core/config/gateway_endpoint.js b/src/core/config/gateway_endpoint.js new file mode 100644 index 0000000..7a1dcc6 --- /dev/null +++ b/src/core/config/gateway_endpoint.js @@ -0,0 +1,54 @@ +// @ts-check + +/** + * @import { HypAwareV2Config } from '../../../collectivus-plugin-kernel-types.d.ts' + */ + +/** + * Resolve the gateway endpoint from the active config's `@hypaware/ai-gateway` + * `listen` directive, for callers that need the URL before the gateway source + * is live in this process (`hyp attach`, the `init` walkthrough, and the + * daemon's attach reconciler when `localEndpoint()` is not yet bindable). It is + * the configured-`listen` fallback shared by all three so they can never derive + * a different port from the same config. + * + * Returns `undefined` when the gateway plugin is absent, its config is not an + * object, or `listen` is missing/malformed — the caller then falls back to a + * placeholder or surfaces the gap. + * + * @param {HypAwareV2Config} config + * @returns {string | undefined} + */ +export function configuredGatewayEndpoint(config) { + const entry = config.plugins?.find((p) => p.name === '@hypaware/ai-gateway') + const cfg = entry?.config + if (!cfg || typeof cfg !== 'object' || Array.isArray(cfg)) return undefined + const listen = /** @type {Record} */ (cfg).listen + if (typeof listen !== 'string') return undefined + return endpointFromListen(listen) +} + +/** + * Turn a `host:port` listen directive into an `http://host:port` URL, + * tolerating bracketed/bare IPv6 hosts. Returns `undefined` for a malformed + * port or empty host. + * + * @param {string} listen + * @returns {string | undefined} + */ +export function endpointFromListen(listen) { + const idx = listen.lastIndexOf(':') + if (idx === -1) return undefined + const rawHost = listen.slice(0, idx) + const rawPort = listen.slice(idx + 1) + const port = Number.parseInt(rawPort, 10) + if (!Number.isInteger(port) || port < 1 || port > 65535 || String(port) !== rawPort) { + return undefined + } + const host = rawHost.startsWith('[') && rawHost.endsWith(']') + ? rawHost.slice(1, -1) + : rawHost + if (host.length === 0) return undefined + const formattedHost = host.includes(':') && !host.startsWith('[') ? `[${host}]` : host + return `http://${formattedHost}:${port}` +} diff --git a/src/core/daemon/runtime.js b/src/core/daemon/runtime.js index b5ddd40..8ea1b15 100644 --- a/src/core/daemon/runtime.js +++ b/src/core/daemon/runtime.js @@ -14,7 +14,9 @@ import { readObservabilityEnv } from '../observability/env.js' import { createConfigControl } from '../config/apply.js' import { buildConfigApplyDeps } from '../config/apply_deps.js' import { createActionReconciler } from '../config/action_reconciler.js' +import { attachHandler } from '../config/action_attach.js' import { backfillHandler } from '../config/action_backfill.js' +import { configuredGatewayEndpoint } from '../config/gateway_endpoint.js' import { bootKernel, resolveLayeredConfigForDaemon } from '../runtime/boot.js' import { createSinkDriver } from '../sinks/driver.js' import { materializeSinks } from '../sinks/materialize.js' @@ -29,9 +31,11 @@ import { openDaemonLog } from './logs.js' import { statusFilePath, writeStatusFile } from './status.js' /** - * @import { JsonObject } from '../../../collectivus-plugin-kernel-types.d.ts' + * @import { AiGatewayCapability, JsonObject } from '../../../collectivus-plugin-kernel-types.d.ts' * @import { KernelRuntime } from '../runtime/activation.js' * @import { BootKernelResult } from '../runtime/types.d.ts' + * @import { ClientDescriptor } from '../plugin_catalog.js' + * @import { ActionHandler } from '../config/types.d.ts' */ /** @@ -48,6 +52,19 @@ import { statusFilePath, writeStatusFile } from './status.js' const DEFAULT_TICK_INTERVAL_MS = 60_000 const MIN_TICK_INTERVAL_MS = 25 +/** + * The client-action handlers the daemon constructs its reconciler with, in the + * order the reconciler runs them: **attach first, then backfill**. The + * reconciler runs handlers serially and `backfillHandler.perform()` awaits a + * (possibly multi-minute) `hyp backfill` subprocess, so attach — an in-process + * settings write — must lead, or live capture is stranded behind the historical + * import. Exported so the ordering is a unit-testable invariant. + * + * @type {ActionHandler[]} + * @ref LLP 0045#module--seam-breakdown-independently-mergeable-tasks [implements] — register [attachHandler, backfillHandler], attach first so live capture leads the backfill subprocess + */ +export const DEFAULT_ACTION_HANDLERS = [attachHandler, backfillHandler] + /** * Exit code a foreground daemon uses to request its own relaunch after * a staged config apply or rollback (EX_TEMPFAIL — "try again"). The @@ -289,21 +306,42 @@ export async function runDaemon(opts = {}) { ) configControl.armProbationWatchdog() - // ----- Client-action reconciler (LLP 0036 / LLP 0037 / LLP 0041) ----- + // ----- Client-action reconciler (LLP 0036 / LLP 0037 / LLP 0041 / LLP 0045) ----- // The daemon is the only host with `configControl`, so a reconciler // attached here is daemon-only by construction: `hyp status` (a plain CLI - // boot) never performs a machine effect. v1 ships one handler, the - // run-once backfill-on-join. Constructed only after boot because a pass - // needs the effective config + the kernel backfill registry. - // @ref LLP 0041#the-reconciler-component [implements] — construct the reconciler with [backfillHandler] in the daemon + // boot) never performs a machine effect. v1 ships two handlers — attach + // (LLP 0045) and the run-once backfill-on-join (LLP 0037). Constructed only + // after boot because a pass needs the effective config + the kernel backfill + // registry, and the attach seam reads the gateway capability the boot bound. + // @ref LLP 0041#the-reconciler-component [implements] — construct the reconciler in the daemon const actionReconciler = opts.actionReconciler ?? createActionReconciler({ stateRoot, - handlers: [backfillHandler], + // Attach first so in-process live-capture wiring starts ahead of the + // (possibly multi-minute) backfill subprocess: the reconciler runs + // handlers serially and `backfillHandler.perform()` awaits its child, so + // attach-first avoids stranding live capture behind the historical import + // (data is order-insensitive — this is purely the latency ordering). + // @ref LLP 0045#module--seam-breakdown-independently-mergeable-tasks [implements] — register [attachHandler, backfillHandler], attach first + handlers: DEFAULT_ACTION_HANDLERS, log: getLogger('action-reconciler'), }) + // The client-action seam the attach handler needs (LLP 0045 §Part 1), + // resolved once from boot now that `startConfiguredSources` has bound the + // gateway source (so `localEndpoint()` is live, not racing): + // - `clientDescriptors` enumerates the client adapters + their owning + // plugins (the static catalog the boot already built); + // - `clients` is the runtime gateway capability used only to invoke a + // client's attach effect, present only when the gateway plugin is enabled; + // - `endpoint` is the local gateway base URL, from `localEndpoint()` with the + // configured-`listen` fallback the CLI uses. + // All three stay undefined on a non-gateway boot, leaving the attach handler + // inert by construction. + // @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — daemon resolves clientDescriptors from the catalog, clients/endpoint from boot.runtime.capabilities when the gateway is enabled + const clientSeam = resolveClientActionSeam({ boot, fileLog }) + /** * Run one reconcile pass against the effective config + backfill registry. * Never throws — a failed handler is surfaced as a `failed` marker by the @@ -334,6 +372,11 @@ export async function runDaemon(opts = {}) { // process.env.HYP_HOME happened to be (LLP 0041 §Run-once flow). // @ref LLP 0041#run-once-flow-backfill-handler [implements] — the child runs against the daemon's resolved HYP_HOME, not process.env env: { ...env, HYP_HOME: hypHome }, + // The client-action seam (LLP 0045 §Part 1) the attach handler reads. + // Undefined on a non-gateway boot — the handler stays inert. + clientDescriptors: clientSeam.clientDescriptors, + clients: clientSeam.clients, + endpoint: clientSeam.endpoint, }) fileLog.info('daemon.reconcile_pass', { hyp_reason: reason, @@ -749,6 +792,58 @@ export function createReconcilePassScheduler({ run, log }) { return { schedule, settle: () => idle } } +/** + * Resolve the client-action seam (LLP 0045 §Part 1) the attach handler reads + * off the reconcile context: the static `clientDescriptors` catalog the boot + * built, and — only when the AI gateway plugin is enabled — the runtime gateway + * capability (`clients`) plus its local base URL (`endpoint`). + * + * The split is load-bearing: `clientDescriptors` carries the owning-plugin field + * the registry lacks (for `desired()`'s "is this client's plugin enabled?" and + * the disk-driven undo's `attachProbe`), while `clients` only *invokes* the + * effect (`getClient(name).attach`). A client adapter requires the gateway + * capability (LLP 0016), so whenever a client plugin is enabled the gateway is + * too; on a non-gateway boot `clients`/`endpoint` stay undefined and the attach + * handler is inert by construction. + * + * `endpoint` prefers the live `localEndpoint()` (the gateway source is already + * bound by the time the reconciler is constructed — `startConfiguredSources` + * ran during boot) and falls back to the configured-`listen` URL, the same + * fallback `hyp attach` uses, so a not-yet-bound gateway still points clients at + * the right port. + * + * @param {{ boot: BootKernelResult, fileLog: ReturnType }} args + * @returns {{ clientDescriptors: Map, clients: AiGatewayCapability | undefined, endpoint: string | undefined }} + * @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — clientDescriptors from the catalog; clients/endpoint from boot.runtime.capabilities, guarded on the gateway capability + */ +function resolveClientActionSeam({ boot, fileLog }) { + const clientDescriptors = boot.clientDescriptors + /** @type {AiGatewayCapability | undefined} */ + let clients + /** @type {string | undefined} */ + let endpoint + + if (boot.runtime.capabilities.has('hypaware.ai-gateway', '^2.0.0')) { + clients = /** @type {AiGatewayCapability} */ ( + boot.runtime.capabilities.require('hyp-core', 'hypaware.ai-gateway', '^2.0.0') + ) + try { + endpoint = clients.localEndpoint() + } catch { + // The gateway source may not be bound yet (e.g. its listen failed); fall + // back to the configured port so attach still writes the right URL. + endpoint = boot.config ? configuredGatewayEndpoint(boot.config) : undefined + } + if (!endpoint) { + fileLog.warn('daemon.attach_endpoint_unresolved', { + hyp_reason: 'no_local_endpoint_and_no_configured_listen', + }) + } + } + + return { clientDescriptors, clients, endpoint } +} + /** * @param {number|undefined} value * @returns {number} diff --git a/src/core/runtime/boot.js b/src/core/runtime/boot.js index 80ab9d3..6af4c16 100644 --- a/src/core/runtime/boot.js +++ b/src/core/runtime/boot.js @@ -246,6 +246,7 @@ export async function bootKernel(opts = {}) { mode, runId, skipped, + clientDescriptors: catalog.clientDescriptors, } } @@ -299,6 +300,7 @@ export async function bootKernel(opts = {}) { mode, runId, skipped, + clientDescriptors: catalog.clientDescriptors, } }, { component: 'kernel' } diff --git a/src/core/runtime/types.d.ts b/src/core/runtime/types.d.ts index f634f3b..28fab33 100644 --- a/src/core/runtime/types.d.ts +++ b/src/core/runtime/types.d.ts @@ -8,6 +8,7 @@ import type { import type { createCommandRegistry } from '../registry/commands.js' import type { ConfigLayerDrop } from '../config/types.d.ts' import type { LoadedManifest, FailedManifest } from '../manifest.d.ts' +import type { ClientDescriptor } from '../plugin_catalog.js' import type { KernelRuntime } from './activation.d.ts' import type { ActivationResult } from './loader.d.ts' @@ -76,6 +77,15 @@ export interface BootKernelResult { runId: string /** Bundled plugins available but not activated this boot. */ skipped: PluginName[] + /** + * Static client→plugin map (`clientName -> { plugin, name, attachProbe? }`) + * derived from the very manifests this boot discovered. The daemon threads + * it onto the client-action reconcile context so the attach handler can + * enumerate `desired()` and reach each descriptor's `attachProbe` for the + * disk-driven undo (LLP 0045 §Part 1). Always present — empty when no plugin + * contributes a client. + */ + clientDescriptors: Map } export interface DiscoverBundledResult { diff --git a/test/core/daemon-reconcile.test.js b/test/core/daemon-reconcile.test.js index 31d3afa..862b603 100644 --- a/test/core/daemon-reconcile.test.js +++ b/test/core/daemon-reconcile.test.js @@ -6,7 +6,13 @@ import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' -import { createReconcilePassScheduler, runDaemon } from '../../src/core/daemon/runtime.js' +import { + createReconcilePassScheduler, + runDaemon, + DEFAULT_ACTION_HANDLERS, +} from '../../src/core/daemon/runtime.js' +import { attachHandler } from '../../src/core/config/action_attach.js' +import { backfillHandler } from '../../src/core/config/action_backfill.js' import { defaultConfigPath } from '../../src/core/config/schema.js' import { centralSeedPath } from '../../src/core/config/apply.js' @@ -161,6 +167,13 @@ test('runDaemon runs the boot already-confirmed pass when a central layer is pre assert.ok(calls[0].config) assert.ok(calls[0].backfills) assert.equal(typeof calls[0].backfills.list, 'function') + // The client-action seam (LLP 0045 §Part 1) is threaded onto every input: + // clientDescriptors always (from the boot catalog), but with no gateway + // plugin enabled the runtime registry + endpoint are undefined, so the + // attach handler is inert by construction. + assert.ok(calls[0].clientDescriptors instanceof Map) + assert.equal(calls[0].clients, undefined) + assert.equal(calls[0].endpoint, undefined) } finally { if (handle) { await handle.stop() @@ -341,3 +354,83 @@ test('the confirmation edge during active probation drives exactly one reconcile await fs.rm(hypHome, { recursive: true, force: true }) } }) + +// --------------------------------------------------------------------------- +// Client-attach daemon wiring (LLP 0045 §Part 7 / T7) +// --------------------------------------------------------------------------- + +test('the daemon registers [attachHandler, backfillHandler] — attach first (LLP 0045 §Part 7)', () => { + // The order is a unit-testable invariant: attach (in-process) must lead the + // backfill subprocess so live-capture wiring is not stranded behind a + // multi-minute historical import. + assert.deepEqual(DEFAULT_ACTION_HANDLERS.map((h) => h.kind), ['attach', 'backfill']) + assert.equal(DEFAULT_ACTION_HANDLERS[0], attachHandler) + assert.equal(DEFAULT_ACTION_HANDLERS[1], backfillHandler) +}) + +test('the daemon resolves clientDescriptors/clients/endpoint from boot when the gateway is enabled', async () => { + const hypHome = await fs.mkdtemp(path.join(os.tmpdir(), 'hypaware-reconcile-gateway-')) + let handle + try { + const stateRoot = path.join(hypHome, 'hypaware') + // A central seed (joined host, no applied slot ⇒ no probation marker) that + // enables the real bundled gateway + claude client adapter, so the boot + // already-confirmed pass fires AND the gateway capability is registered. + const seedPath = centralSeedPath(stateRoot) + await fs.mkdir(path.dirname(seedPath), { recursive: true }) + await fs.writeFile( + seedPath, + JSON.stringify({ + version: 2, + plugins: [ + { + name: '@hypaware/ai-gateway', + config: { + listen: '127.0.0.1:0', + upstreams: [ + { name: 'anthropic', base_url: 'https://api.anthropic.com', path_prefix: '/' }, + ], + }, + }, + { name: '@hypaware/claude' }, + ], + }) + '\n' + ) + + const configPath = defaultConfigPath(hypHome) + await fs.mkdir(path.dirname(configPath), { recursive: true }) + await fs.writeFile(configPath, JSON.stringify({ version: 2, plugins: [] }) + '\n') + + const { reconciler, calls } = makeFakeReconciler() + handle = await runDaemon({ + hypHome, + configPath, + env: { ...process.env, HYP_HOME: hypHome }, + runId: 'reconcile-gateway-test', + tickIntervalMs: 0, + installSignalHandlers: false, + actionReconciler: reconciler, + }) + + await waitFor(() => calls.length === 1) + // The static client→plugin catalog is threaded and names the claude adapter. + assert.ok(calls[0].clientDescriptors instanceof Map) + assert.ok( + calls[0].clientDescriptors?.get('claude'), + 'clientDescriptors carries the claude descriptor' + ) + // The runtime gateway capability is present (clients) and exposes the + // attach-invocation surface the handler reaches perform() through. + assert.ok(calls[0].clients, 'the gateway capability is threaded as clients') + assert.equal(typeof calls[0].clients?.getClient, 'function') + // The endpoint resolved from the live gateway source (bound on an ephemeral + // port before the reconciler was constructed). + assert.match(String(calls[0].endpoint), /^http:\/\/127\.0\.0\.1:\d+$/) + } finally { + if (handle) { + await handle.stop() + await handle.done + } + await fs.rm(hypHome, { recursive: true, force: true }) + } +}) From 5b972c87b99019d561f800fcbdc562cad5e91ebd Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 15:30:11 -0700 Subject: [PATCH 11/15] LLP 0045: flip Status Accepted -> Active (client attach shipped) Marks the client-attach implementation design as built-and-merged now that the whole change set (T1-T9) has landed. Active = shipped marker per the project lifecycle (sibling design LLP 0041 is already Active). Co-Authored-By: Claude Opus 4.8 (1M context) Task-Id: T10 --- llp/0045-client-attach.design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llp/0045-client-attach.design.md b/llp/0045-client-attach.design.md index 643a39c..e10ded8 100644 --- a/llp/0045-client-attach.design.md +++ b/llp/0045-client-attach.design.md @@ -1,7 +1,7 @@ # LLP 0045: Client attach on join — implementation design **Type:** design -**Status:** Accepted +**Status:** Active **Systems:** Config, Daemon, Onboarding, Sources, Gateway **Author:** Phil / Claude **Date:** 2026-06-26 From 7432284a1da605093f71de362be7d3af4eeab2eb Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 20:17:56 -0700 Subject: [PATCH 12/15] client-attach: legacy-marker detach fallback; descriptor-map manual detach; bundled+installed descriptors; type+doc fixes (#179 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dual-review request_changes fixes for PR #179 (integration/client-attach): 1. Legacy-marker half-reversal: detachJsonMarker now falls back to the pre-self-describing convention when marker.managed is absent — removes ANTHROPIC_BASE_URL at the recorded port and strips claude-hook session-context hooks, so a manual `hyp detach` after upgrade fully reverses with nothing orphaned. Logic moved from the now-retired claude-adapter detach() into the one core undo. 2. Manual detach no longer gated on the live gateway: runClientLifecycle resolves/validates the detach target from the bundled+installed descriptor map, so a dropped/unloaded adapter's on-disk attach still reverses. 3. buildClientDescriptorMap now built from bundled+installed (matching boot/status), so installed (non-bundled) client adapters resolve. 4. DetachFromDiskResult moved from a @typedef to an interface in config/types.d.ts, imported via @import. 5. Relabel "§Part 7" prose to "§Module / seam breakdown item 7". Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0178wfuBrVQHwPbsHAtcE7YS --- .../plugins-workspace/claude/src/settings.js | 88 +----------- src/core/cli/core_commands.js | 90 +++++++++--- src/core/config/action_attach.js | 5 +- src/core/config/client_detach_disk.js | 136 ++++++++++++++++-- src/core/config/types.d.ts | 19 ++- test/core/client-detach-disk.test.js | 73 ++++++++++ test/core/daemon-reconcile.test.js | 4 +- test/core/integration.test.js | 99 ++++++++++++- 8 files changed, 394 insertions(+), 120 deletions(-) diff --git a/hypaware-core/plugins-workspace/claude/src/settings.js b/hypaware-core/plugins-workspace/claude/src/settings.js index fb14224..38d2191 100644 --- a/hypaware-core/plugins-workspace/claude/src/settings.js +++ b/hypaware-core/plugins-workspace/claude/src/settings.js @@ -6,15 +6,17 @@ import os from 'node:os' import path from 'node:path' /** - * Claude Code settings.json attach/detach writer. Ported from the + * Claude Code settings.json attach writer. Ported from the * Collectivus donor `src/claude-code/settings.js`, with the managed * marker key renamed `_collectivus` → `_hypaware` and the embedded * hook command pointed at `hyp` instead of `ctvs`. * * Writes are atomic (temp file + rename) and gated on mtime so a * concurrent edit is detected instead of silently overwritten. The - * `_hypaware` marker is what `detach()` keys off to know which keys - * it inserted and is safe to remove. + * `_hypaware` marker is the self-describing undo record the single core + * undo (`detachClientFromDisk`, LLP 0045 §Part 3) replays — there is no + * adapter `detach()`; the reverse lives in core so it survives the + * plugin being unloaded (legacy pre-record markers included). * * The marker is also a **self-describing undo record**: it records * `prev_base_url` (the restore target) and the managed @@ -25,7 +27,7 @@ import path from 'node:path' */ /** - * @import { ClaudeAttachOptions, ClaudeAttachResult, ClaudeDetachOptions, ClaudeDetachResult } from './types.d.ts' + * @import { ClaudeAttachOptions, ClaudeAttachResult } from './types.d.ts' * @import { FileHandle } from 'node:fs/promises' */ @@ -123,53 +125,6 @@ export async function attach(opts) { return result } -/** - * Reverse a previous `attach`. No-op when settings.json is absent or - * has no `_hypaware` marker. Removes `env.ANTHROPIC_BASE_URL` only - * when it still matches the recorded port; otherwise leaves it and - * surfaces a warning. - * - * @param {ClaudeDetachOptions} [opts] - * @returns {Promise} - */ -export async function detach(opts = {}) { - const { settingsPath = defaultSettingsPath() } = opts - const { value, existed, mtimeMs } = await readSettings(settingsPath) - - if (!existed) return { changed: false } - - const marker = value[MARKER_KEY] - if (!isPlainObject(marker)) return { changed: false } - - const markerPort = typeof marker.port === 'number' ? marker.port : undefined - delete value[MARKER_KEY] - removeSessionContextHooks(value) - - /** @type {string | undefined} */ - let removed - /** @type {string | undefined} */ - let warning - if (isPlainObject(value.env)) { - const env = /** @type {Record} */ (value.env) - const current = env.ANTHROPIC_BASE_URL - if (markerPort !== undefined && current === `http://127.0.0.1:${markerPort}`) { - removed = /** @type {string} */ (current) - delete env.ANTHROPIC_BASE_URL - } else if (typeof current === 'string') { - warning = 'ANTHROPIC_BASE_URL was overridden externally; leaving in place' - } - if (Object.keys(env).length === 0) delete value.env - } - - await writeAtomic(settingsPath, value, mtimeMs) - - /** @type {ClaudeDetachResult} */ - const result = { changed: true } - if (removed !== undefined) result.removed = removed - if (warning !== undefined) result.warning = warning - return result -} - /** * @param {string} settingsPath * @returns {Promise<{ value: Record, existed: boolean, mtimeMs: number | undefined }>} @@ -305,32 +260,6 @@ function installSessionContextHooks(value, command) { } } -/** - * @param {Record} value - */ -function removeSessionContextHooks(value) { - const hooksRoot = value.hooks - if (!isPlainObject(hooksRoot)) return - for (const event of managedHookEvents()) { - const existing = hooksRoot[event] - if (!Array.isArray(existing)) continue - const groups = existing - .filter((group) => !isManagedHookGroup(group)) - .map(removeManagedHandlers) - .filter((group) => !isEmptyHookGroup(group)) - if (groups.length > 0) { - hooksRoot[event] = groups - } else { - delete hooksRoot[event] - } - } - if (Object.keys(hooksRoot).length === 0) delete value.hooks -} - -function managedHookEvents() { - return [...new Set(MANAGED_HOOK_SPECS.map((spec) => spec.event))] -} - /** * The managed session-context hook entries this attach installs, * recorded into the marker's undo record so the core undo can strip @@ -368,11 +297,6 @@ function isManagedHookGroup(group) { handlers.every(isManagedHookHandler) } -/** @param {unknown} group */ -function isEmptyHookGroup(group) { - return isPlainObject(group) && Array.isArray(group.hooks) && group.hooks.length === 0 -} - /** @param {unknown} handler */ function isManagedHookHandler(handler) { if (!isPlainObject(handler)) return false diff --git a/src/core/cli/core_commands.js b/src/core/cli/core_commands.js index 963b996..d78cc58 100644 --- a/src/core/cli/core_commands.js +++ b/src/core/cli/core_commands.js @@ -3257,31 +3257,39 @@ async function runClientLifecycle(action, argv, ctx) { /** @type {AiGatewayCapability} */ const gateway = ctx.capabilities.require('hyp-core', 'hypaware.ai-gateway', '^2.0.0') - const clientNames = expandClientName(parsed.client, gateway) + // Detach is the single core disk-driven undo, resolved per client via its + // static descriptor (owning plugin + attach_probe) — not a per-adapter + // detach() hook and not the live gateway registry, so it reverses an on-disk + // attach even for a client whose adapter has been dropped/unloaded (LLP 0045 + // §Part 3). Build the bundled+installed descriptor map once and resolve from + // it; attach still needs the live adapter, so it resolves from the gateway. + const clientDescriptors = action === 'detach' ? await buildClientDescriptorMap(ctx) : undefined + + const clientNames = action === 'detach' + ? expandDetachClientNames(parsed.client, clientDescriptors) + : expandClientName(parsed.client, gateway) if (clientNames.length === 0) { + const known = action === 'detach' + ? [...(clientDescriptors?.keys() ?? [])] + : gateway.listClients().map((c) => c.name) ctx.stderr.write( - `error: unknown client '${parsed.client}'. Registered clients: ${ - gateway.listClients().map((c) => c.name).join(', ') || '(none)' - }\n` + `error: unknown client '${parsed.client}'. ${ + action === 'detach' ? 'Known' : 'Registered' + } clients: ${known.join(', ') || '(none)'}\n` ) return 1 } - // Detach is the single core disk-driven undo, resolved per client via - // its static descriptor (owning plugin + attach_probe) — not a - // per-adapter detach() hook (LLP 0045 §Part 3). Build the map once. - const clientDescriptors = action === 'detach' ? await buildClientDescriptorMap() : undefined - let exitCode = 0 for (const name of clientNames) { - const client = gateway.getClient(name) - if (!client) { - ctx.stderr.write(`error: unknown client '${name}'\n`) - exitCode = 1 - continue - } try { if (action === 'attach') { + const client = gateway.getClient(name) + if (!client) { + ctx.stderr.write(`error: unknown client '${name}'\n`) + exitCode = 1 + continue + } // In dry-run mode the gateway source may not be started yet, // so `localEndpoint()` could throw. Fall back to a placeholder // endpoint — adapters are expected to short-circuit before @@ -3311,9 +3319,15 @@ async function runClientLifecycle(action, argv, ctx) { json: parsed.json, }) } else { + const descriptor = clientDescriptors?.get(name) + if (!descriptor) { + ctx.stderr.write(`error: unknown client '${name}'\n`) + exitCode = 1 + continue + } await detachClientViaCore({ name, - descriptor: clientDescriptors?.get(name), + descriptor, dryRun: parsed.dryRun, json: parsed.json, ctx, @@ -3532,6 +3546,22 @@ function expandClientName(requested, gateway) { return [requested] } +/** + * Resolve `--client all` to every known client name from the descriptor map + * (bundled+installed) for the disk-driven detach; otherwise return the + * requested name verbatim (validated against the map at the call site). Detach + * must not consult the live gateway registry — a client whose adapter was + * dropped/unloaded still has an on-disk attach to reverse (LLP 0045 §Part 3). + * + * @param {string} requested + * @param {Map | undefined} descriptors + * @returns {string[]} + */ +function expandDetachClientNames(requested, descriptors) { + if (requested === 'all') return [...(descriptors?.keys() ?? [])] + return [requested] +} + /** * @param {string[]} _argv * @param {CommandRunContext} ctx @@ -3571,7 +3601,7 @@ async function runSkillsInstall(argv, ctx) { return 1 } - const descriptorMap = await buildClientDescriptorMap() + const descriptorMap = await buildClientDescriptorMap(ctx) let count = 0 for (const skill of skills) { @@ -3629,7 +3659,7 @@ async function runAgentsInstall(argv, ctx) { return 1 } - const descriptorMap = await buildClientDescriptorMap() + const descriptorMap = await buildClientDescriptorMap(ctx) let count = 0 for (const agent of agents) { @@ -3668,18 +3698,36 @@ async function runAgentsInstall(argv, ctx) { * manifests. This avoids hardcoding `.claude/skills` / `.codex/skills` * / `.claude/agents` in core. * + * Built from the same **bundled + installed** catalog that `boot.js` and + * `status.js` use, so an installed (non-bundled) client adapter that can + * attach-on-join is also resolvable here — its `hyp detach` / skill / agent + * install must not silently miss the descriptor. + * + * @param {CommandRunContext} ctx * @returns {Promise>} */ -async function buildClientDescriptorMap() { +async function buildClientDescriptorMap(ctx) { /** @type {Map} */ const map = new Map() + /** @type {import('../manifest.js').LoadedManifest[]} */ + let bundledLoaded = [] + /** @type {import('../manifest.js').LoadedManifest[]} */ + let installedLoaded = [] try { const bundled = await discoverBundledPlugins() - const catalog = buildPluginCatalog([...bundled.loaded, ...bundled.excluded]) + bundledLoaded = [...bundled.loaded, ...bundled.excluded] + } catch { /* bundled discovery failure is non-fatal */ } + try { + const stateDir = pluginStateDir(ctx) + const installed = await discoverInstalledPlugins({ stateDir }) + installedLoaded = installed.loaded + } catch { /* installed discovery failure is non-fatal */ } + try { + const catalog = buildPluginCatalog(bundledLoaded, installedLoaded) for (const [clientName, descriptor] of catalog.clientDescriptors) { map.set(clientName, descriptor) } - } catch { /* discovery failure → empty map → warnings per contribution */ } + } catch { /* catalog build failure → empty map → warnings per contribution */ } return map } diff --git a/src/core/config/action_attach.js b/src/core/config/action_attach.js index 62b1a6a..0667a56 100644 --- a/src/core/config/action_attach.js +++ b/src/core/config/action_attach.js @@ -230,8 +230,9 @@ export function createAttachHandler(opts = {}) { * The default `attachHandler` the daemon registers the reconciler with — first * in the `[attachHandler, backfillHandler]` order so in-process live-capture * wiring starts ahead of the (possibly multi-minute) backfill subprocess - * (LLP 0045 §Part 7). Uses the real `detachClientFromDisk`; tests build their - * own via {@link createAttachHandler} with an injected `detach`. + * (LLP 0045 §Module / seam breakdown item 7). Uses the real + * `detachClientFromDisk`; tests build their own via {@link createAttachHandler} + * with an injected `detach`. * * @type {ActionHandler} */ diff --git a/src/core/config/client_detach_disk.js b/src/core/config/client_detach_disk.js index 10cab06..2ec2252 100644 --- a/src/core/config/client_detach_disk.js +++ b/src/core/config/client_detach_disk.js @@ -10,6 +10,7 @@ import { resolveClientSettingsPath } from '../daemon/client_settings_path.js' /** * @import { FileHandle } from 'node:fs/promises' * @import { ClientDescriptor } from '../plugin_catalog.js' + * @import { DetachFromDiskResult } from './types.d.ts' */ /** @@ -68,15 +69,6 @@ export class ClientDetachError extends Error { } } -/** - * @typedef {object} DetachFromDiskResult - * @property {boolean} changed True when the settings file was rewritten. - * @property {string} [settingsPath] The resolved settings path (when one exists). - * @property {string} [removed] The managed value deleted (e.g. the gateway base URL) when there was no prior to restore. - * @property {string} [restoredValue] The prior value restored from the undo record. - * @property {string} [warning] Set when the managed value was overridden externally and left in place. - */ - /** * Reverse a client's attach from disk, driven by the descriptor's * `attachProbe` and the settings-file marker. No-op (`{ changed: false }`) when @@ -125,6 +117,24 @@ async function detachJsonMarker({ settingsPath, markerKey, fs }) { const marker = value[markerKey] if (!isPlainObject(marker)) return { changed: false, settingsPath } + // Pre-upgrade markers have the legacy shape {attached_at,version,port, + // state_file} with no self-describing `managed` undo record. There is no + // record to replay, so reverse them by the original (now-retired) convention + // instead of just deleting the marker — otherwise env.ANTHROPIC_BASE_URL and + // the `hyp claude-hook session-context` entries it wrote would orphan, and the + // detach is non-retryable once the marker is gone. + // @ref LLP 0045#part-3--reverse-runs-from-disk-the-marker-is-a-self-describing-undo-record [constrained-by] — legacy markers predate the undo record; fall back to the convention attach used before it + if (!isPlainObject(marker.managed)) { + return await detachLegacyJsonMarker({ + settingsPath, + markerKey, + value, + marker, + mtimeMs: read.mtimeMs, + fs, + }) + } + const managed = isPlainObject(marker.managed) ? marker.managed : {} const managedEnv = isPlainObject(managed.env) ? managed.env : {} const hookEntries = Array.isArray(managed.hooks) ? managed.hooks : [] @@ -239,6 +249,114 @@ function isManagedHandler(handler, command) { return isPlainObject(handler) && handler.type === 'command' && handler.command === command } +/* ----------------------------- legacy JSON marker ---------------------------- */ + +const LEGACY_CLAUDE_HOOK_PATTERN = /\bclaude-hook\s+session-context\b/ + +/** + * Reverse a pre-upgrade legacy `json` marker — the old Claude marker shape + * `{attached_at,version,port,state_file}` that predates the self-describing + * `managed` undo record. We can't replay a record the marker never wrote, so we + * fall back to the convention `attach()` used before the record existed: + * remove `env.ANTHROPIC_BASE_URL` only when it still equals the recorded + * `http://127.0.0.1:${port}` gateway URL (never clobbering a later user edit), + * and strip the session-context hooks by the `claude-hook session-context` + * command pattern. Legacy JSON markers were only ever written by Claude, so the + * key/pattern are safe to assume here. Moved from the retired claude-adapter + * `detach()` so the one core undo owns this reversal too. + * + * @param {{ + * settingsPath: string, + * markerKey: string, + * value: Record, + * marker: Record, + * mtimeMs: number | undefined, + * fs: typeof fsp, + * }} args + * @returns {Promise} + */ +async function detachLegacyJsonMarker({ settingsPath, markerKey, value, marker, mtimeMs, fs }) { + const markerPort = typeof marker.port === 'number' ? marker.port : undefined + + delete value[markerKey] + stripLegacyClaudeHooks(value) + + /** @type {string | undefined} */ + let removed + /** @type {string | undefined} */ + let warning + if (isPlainObject(value.env)) { + const envObj = /** @type {Record} */ (value.env) + const current = envObj.ANTHROPIC_BASE_URL + if (markerPort !== undefined && current === `http://127.0.0.1:${markerPort}`) { + removed = typeof current === 'string' ? current : String(current) + delete envObj.ANTHROPIC_BASE_URL + } else if (typeof current === 'string') { + warning = 'ANTHROPIC_BASE_URL was overridden externally; leaving in place' + } + if (Object.keys(envObj).length === 0) delete value.env + } + + await writeJsonAtomic(settingsPath, value, mtimeMs, fs) + + /** @type {DetachFromDiskResult} */ + const result = { changed: true, settingsPath } + if (removed !== undefined) result.removed = removed + if (warning !== undefined) result.warning = warning + return result +} + +/** + * Strip the legacy Claude session-context hooks — matched by the + * `claude-hook session-context` command pattern rather than the marker's undo + * record (a legacy marker recorded no hook entries). Empty groups, emptied + * event arrays, and an emptied `hooks` root are pruned, so no orphaned `hyp …` + * hooks survive. Preserves a user's own non-managed handlers for the same event. + * + * @param {Record} value + */ +function stripLegacyClaudeHooks(value) { + const hooksRoot = value.hooks + if (!isPlainObject(hooksRoot)) return + + for (const event of Object.keys(hooksRoot)) { + const groups = hooksRoot[event] + if (!Array.isArray(groups)) continue + + /** @type {unknown[]} */ + const nextGroups = [] + for (const group of groups) { + if (!isPlainObject(group) || !Array.isArray(group.hooks)) { + nextGroups.push(group) + continue + } + const keptHandlers = group.hooks.filter((h) => !isLegacyClaudeHandler(h)) + if (keptHandlers.length === group.hooks.length) { + nextGroups.push(group) // nothing matched — leave untouched + } else if (keptHandlers.length > 0) { + nextGroups.push({ ...group, hooks: keptHandlers }) + } + // else: the group held only legacy managed handlers — drop it entirely. + } + + if (nextGroups.length > 0) { + hooksRoot[event] = nextGroups + } else { + delete hooksRoot[event] + } + } + + if (Object.keys(hooksRoot).length === 0) delete value.hooks +} + +/** @param {unknown} handler */ +function isLegacyClaudeHandler(handler) { + return isPlainObject(handler) && + handler.type === 'command' && + typeof handler.command === 'string' && + LEGACY_CLAUDE_HOOK_PATTERN.test(handler.command) +} + /* ------------------------------- TOML format ------------------------------ */ /** diff --git a/src/core/config/types.d.ts b/src/core/config/types.d.ts index 31c3aa2..4ff02c1 100644 --- a/src/core/config/types.d.ts +++ b/src/core/config/types.d.ts @@ -14,7 +14,6 @@ import type { AiGatewayCapability, } from '../../../collectivus-plugin-kernel-types.d.ts' import type { ClientDescriptor } from '../plugin_catalog.js' -import type { DetachFromDiskResult } from './client_detach_disk.js' /** * Outcome of the `init` overwrite guard (LLP 0031). `proceed` is true @@ -575,6 +574,24 @@ export interface CreateBackfillHandlerOptions { * real implementation accepts more (an injectable `fs` / `homeDir`), so it is * assignable to this narrower type. */ +/** + * Outcome of the single core disk-driven undo (`detachClientFromDisk`, LLP 0045 + * §Part 3). Defined here (not as a `@typedef` in the implementation) so it is a + * shared `interface` other modules import via `@import`. + */ +export interface DetachFromDiskResult { + /** True when the settings file was rewritten. */ + changed: boolean + /** The resolved settings path (when one exists). */ + settingsPath?: string + /** The managed value deleted (e.g. the gateway base URL) when there was no prior to restore. */ + removed?: string + /** The prior value restored from the undo record. */ + restoredValue?: string + /** Set when the managed value was overridden externally and left in place. */ + warning?: string +} + export type ClientDetachFromDisk = (args: { descriptor: ClientDescriptor homeDir?: string diff --git a/test/core/client-detach-disk.test.js b/test/core/client-detach-disk.test.js index 3a2f689..4bed057 100644 --- a/test/core/client-detach-disk.test.js +++ b/test/core/client-detach-disk.test.js @@ -159,6 +159,79 @@ test('claude undo strips marker + managed keys/hooks from a hand-written fixture } }) +test('claude undo of a LEGACY pre-upgrade marker (no managed record) detaches fully', async () => { + const home = await stageHome() + try { + // The old marker shape attach wrote before the self-describing `managed` + // undo record existed: {attached_at,version,port,state_file} only, no + // `managed`/`prev_base_url`. Reached by a manual `hyp detach` after upgrade. + // The undo must fall back to the original convention — remove the gateway + // base URL, strip the `claude-hook session-context` hooks — so nothing is + // left orphaned (deleting the marker alone is non-retryable half-reversal). + const command = "hyp claude-hook session-context --state-file '/abs/session-context.jsonl'" + const fixture = { + env: { ANTHROPIC_API_KEY: 'sk-x', ANTHROPIC_BASE_URL: 'http://127.0.0.1:4123' }, + hooks: { + SessionStart: [{ hooks: [{ type: 'command', command }] }], + CwdChanged: [{ hooks: [{ type: 'command', command }] }], + UserPromptSubmit: [{ hooks: [{ type: 'command', command }] }], + PostToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command }] }], + }, + _hypaware: { + attached_at: '2026-06-26T00:00:00.000Z', + version: '0.2.0', + port: 4123, + state_file: '/abs/session-context.jsonl', + }, + } + const settingsPath = await writeClaudeSettings(home, JSON.stringify(fixture, null, 2) + '\n') + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.equal(result.removed, 'http://127.0.0.1:4123') + assert.equal('restoredValue' in result, false) // legacy markers recorded no prior + + const raw = await fs.readFile(settingsPath, 'utf8') + const parsed = JSON.parse(raw) + assert.equal('_hypaware' in parsed, false) // marker gone + assert.equal('ANTHROPIC_BASE_URL' in (parsed.env ?? {}), false) // no orphaned base URL + assert.equal(parsed.env.ANTHROPIC_API_KEY, 'sk-x') // unrelated env preserved + assert.equal('hooks' in parsed, false) // every managed hook group pruned + assert.equal(raw.includes('claude-hook'), false) // no orphaned hyp hooks + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + +test('claude undo of a LEGACY marker preserves a user hook and an externally-overridden base URL', async () => { + const home = await stageHome() + try { + const command = "hyp claude-hook session-context --state-file '/abs/session-context.jsonl'" + const fixture = { + env: { ANTHROPIC_BASE_URL: 'https://someone-else.example' }, // user re-pointed it + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'echo hello' }] }, // user's own + { hooks: [{ type: 'command', command }] }, // ours (legacy-installed) + ], + }, + _hypaware: { version: '0.2.0', port: 4123 }, // legacy shape, no managed record + } + const settingsPath = await writeClaudeSettings(home, JSON.stringify(fixture, null, 2) + '\n') + + const result = await detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home }) + assert.equal(result.changed, true) + assert.match(String(result.warning), /overridden externally/) + + const parsed = JSON.parse(await fs.readFile(settingsPath, 'utf8')) + assert.equal('_hypaware' in parsed, false) // marker still removed + assert.equal(parsed.env.ANTHROPIC_BASE_URL, 'https://someone-else.example') // user value untouched + assert.deepEqual(parsed.hooks.SessionStart, [{ hooks: [{ type: 'command', command: 'echo hello' }] }]) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) + test('claude undo leaves an externally-overridden base URL in place with a warning', async () => { const home = await stageHome() try { diff --git a/test/core/daemon-reconcile.test.js b/test/core/daemon-reconcile.test.js index 862b603..075d367 100644 --- a/test/core/daemon-reconcile.test.js +++ b/test/core/daemon-reconcile.test.js @@ -356,10 +356,10 @@ test('the confirmation edge during active probation drives exactly one reconcile }) // --------------------------------------------------------------------------- -// Client-attach daemon wiring (LLP 0045 §Part 7 / T7) +// Client-attach daemon wiring (LLP 0045 §Module / seam breakdown item 7 / T7) // --------------------------------------------------------------------------- -test('the daemon registers [attachHandler, backfillHandler] — attach first (LLP 0045 §Part 7)', () => { +test('the daemon registers [attachHandler, backfillHandler] — attach first (LLP 0045 §Module / seam breakdown item 7)', () => { // The order is a unit-testable invariant: attach (in-process) must lead the // backfill subprocess so live-capture wiring is not stranded behind a // multi-minute historical import. diff --git a/test/core/integration.test.js b/test/core/integration.test.js index 293c6e8..51684b6 100644 --- a/test/core/integration.test.js +++ b/test/core/integration.test.js @@ -11,16 +11,26 @@ import { createCommandRegistry } from '../../src/core/registry/commands.js' import { registerCoreCommands } from '../../src/core/cli/core_commands.js' import { createKernelRuntime } from '../../src/core/runtime/activation.js' import { centralSeedPath } from '../../src/core/config/apply.js' +import { writeLock } from '../../src/core/plugin_install/lock.js' -/** A fake ai-gateway kernel that records attach/detach calls and emits JSON. */ -function fakeClientKernel() { +/** + * A fake ai-gateway kernel that records attach/detach calls and emits JSON. + * + * `clientNames` controls which client adapters the *live* gateway registry + * exposes; pass `[]` to model an adapter that has been dropped/unloaded while + * the gateway capability itself is still active (the disk-driven detach must + * still resolve such a client from the bundled descriptor map). + * + * @param {{ clientNames?: string[] }} [opts] + */ +function fakeClientKernel({ clientNames = ['claude', 'codex'] } = {}) { const registry = createCommandRegistry() registerCoreCommands(registry) const kernel = createKernelRuntime({ commandRegistry: registry }) /** @type {Array<{ action: string, client: string, json: boolean }>} */ const calls = [] const clients = new Map( - ['claude', 'codex'].map((name) => [ + clientNames.map((name) => [ name, { name, @@ -120,6 +130,89 @@ test('detach returns the parsed structured result (core disk undo)', async () => assert.deepEqual(calls, []) }) +test('detach reverses a client whose adapter was dropped from the live gateway (LLP 0045 §Part 3)', async () => { + // The ai-gateway capability is present, but the codex adapter has been + // dropped/unloaded — the live registry exposes no clients. Detach must still + // resolve the target from the bundled+installed descriptor map and run the + // disk-driven undo; it is NOT gated on gateway.getClient. + const { registry, kernel, calls } = fakeClientKernel({ clientNames: [] }) + const home = await freshHome() + const result = await detach('codex', { + hypHome: home, + env: { ...process.env, HOME: home, CODEX_HOME: home }, + // @ts-expect-error test-only kernel injection + registry, + kernel, + }) + // Resolved + ran (no marker on the temp tree → clean no-op) rather than + // failing "unknown client", and the (retired) adapter detach() is untouched. + assert.equal(result.status, 'ok') + assert.equal(result.action, 'detach') + assert.equal(result.client, 'codex') + assert.equal(result.changed, false) + assert.deepEqual(calls, []) +}) + +test('detach resolves an INSTALLED (non-bundled) client adapter from the bundled+installed descriptor map (LLP 0045 §Part 3)', async () => { + // buildClientDescriptorMap() was bundled-only while boot/status use + // bundled+installed. Stage an installed client plugin with an attach_probe and + // prove `hyp detach ` resolves its descriptor and runs the disk undo + // — otherwise it would throw "no client descriptor". + const { registry, kernel } = fakeClientKernel() + const hypHome = await freshHome() + const stateDir = path.join(hypHome, 'hypaware') + const installDir = path.join(stateDir, 'plugins', 'widget') + await fs.mkdir(installDir, { recursive: true }) + await fs.writeFile( + path.join(installDir, 'hypaware.plugin.json'), + JSON.stringify({ + schema_version: 1, + name: '@acme/widget', + version: '1.0.0', + hypaware_api: '^1.0.0', + runtime: 'node', + entrypoint: './index.js', + requires: { capabilities: { 'hypaware.ai-gateway': '^2.0.0' } }, + contributes: { + client: { + name: 'widget', + skill_dir: '.widget/skills', + attach_probe: { format: 'json', settings_file: '.widget/settings.json', marker_key: '_hypaware' }, + }, + }, + }, null, 2) + ) + await fs.writeFile(path.join(installDir, 'index.js'), 'export async function activate() {}\n') + await writeLock(stateDir, { + schema_version: 1, + plugins: { + '@acme/widget': { + name: '@acme/widget', + version: '1.0.0', + source: { kind: 'local-dir', raw: installDir, path: installDir }, + install_dir: installDir, + content_hash: 'a'.repeat(64), + manifest_hash: 'b'.repeat(64), + installed_at: '2026-06-26T00:00:00.000Z', + }, + }, + }) + + const result = await detach('widget', { + hypHome, + env: { ...process.env, HOME: hypHome, HYP_HOME: hypHome }, + // @ts-expect-error test-only kernel injection + registry, + kernel, + }) + // Resolved from the *installed* catalog (not bundled, not the live gateway) + // and ran the disk undo to a clean no-op rather than failing "unknown client". + assert.equal(result.status, 'ok') + assert.equal(result.action, 'detach') + assert.equal(result.client, 'widget') + assert.equal(result.changed, false) +}) + test('attach throws HypAwareCommandError for an unknown client', async () => { const { registry, kernel } = fakeClientKernel() const hypHome = await freshHome() From e32353e586fd7568d8d7bd2fd8c5c03bca74f64b Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 21:17:22 -0700 Subject: [PATCH 13/15] client-attach: detach works without gateway; TOCTOU stat-before-read; drop dead types; hoist import types; document stable client seam (#179 round-2) Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0178wfuBrVQHwPbsHAtcE7YS --- .../plugins-workspace/claude/src/types.d.ts | 8 - src/core/cli/core_commands.js | 145 ++++++++++-------- src/core/config/client_detach_disk.js | 18 ++- src/core/daemon/runtime.js | 7 + test/core/integration.test.js | 55 +++++++ 5 files changed, 154 insertions(+), 79 deletions(-) diff --git a/hypaware-core/plugins-workspace/claude/src/types.d.ts b/hypaware-core/plugins-workspace/claude/src/types.d.ts index 6b4555a..231d553 100644 --- a/hypaware-core/plugins-workspace/claude/src/types.d.ts +++ b/hypaware-core/plugins-workspace/claude/src/types.d.ts @@ -80,11 +80,3 @@ export interface ClaudeAttachOptions { } export type ClaudeAttachResult = { changed: true; prevValue?: string } | { changed: false } - -export interface ClaudeDetachOptions { - settingsPath?: string -} - -export type ClaudeDetachResult = - | { changed: true; removed?: string; warning?: string } - | { changed: false } diff --git a/src/core/cli/core_commands.js b/src/core/cli/core_commands.js index d78cc58..d8b119a 100644 --- a/src/core/cli/core_commands.js +++ b/src/core/cli/core_commands.js @@ -63,6 +63,7 @@ import { SCAFFOLD_KINDS, scaffoldPlugin } from '../plugin_doctor/scaffold.js' /** * @import { AiGatewayCapability, CommandRegistration, CommandRunContext, HypAwareV2Config, PluginName } from '../../../collectivus-plugin-kernel-types.d.ts' * @import { ClientDescriptor } from '../plugin_catalog.js' + * @import { LoadedManifest } from '../manifest.js' * @import { ExtendedQueryStorageService } from '../cache/types.d.ts' * @import { PluginMetadata } from '../config/types.d.ts' * @import { DaemonInstallOptions, HypAwareStatusReport, ServiceState } from '../daemon/types.d.ts' @@ -3220,6 +3221,50 @@ async function runClientLifecycle(action, argv, ctx) { return 2 } + // Detach is the single core, disk-driven undo (LLP 0045 §Part 3): it reverses + // an on-disk attach from the static client descriptor map (owning plugin + + // attach_probe), never the live gateway registry. So it must keep working + // with the @hypaware/ai-gateway capability absent/unloaded — resolve and + // reverse here, AHEAD of the gateway gate. Attach genuinely needs the live + // adapter, so it stays gated below. + if (action === 'detach') { + const clientDescriptors = await buildClientDescriptorMap(ctx) + const clientNames = expandDetachClientNames(parsed.client, clientDescriptors) + if (clientNames.length === 0) { + const known = [...clientDescriptors.keys()] + ctx.stderr.write( + `error: unknown client '${parsed.client}'. Known clients: ${known.join(', ') || '(none)'}\n` + ) + return 1 + } + let exitCode = 0 + for (const name of clientNames) { + try { + const descriptor = clientDescriptors.get(name) + if (!descriptor) { + ctx.stderr.write(`error: unknown client '${name}'\n`) + exitCode = 1 + continue + } + await detachClientViaCore({ + name, + descriptor, + dryRun: parsed.dryRun, + json: parsed.json, + ctx, + }) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + ctx.stderr.write(`error: detach client '${name}' failed: ${message}\n`) + exitCode = 1 + } + } + return exitCode + } + + // Attach dispatches to the per-adapter attach() hook and threads the + // gateway's localEndpoint(), so it requires the live @hypaware/ai-gateway + // capability. if (!ctx.capabilities.has('hypaware.ai-gateway')) { await withSpan( `client.${action}`, @@ -3257,25 +3302,11 @@ async function runClientLifecycle(action, argv, ctx) { /** @type {AiGatewayCapability} */ const gateway = ctx.capabilities.require('hyp-core', 'hypaware.ai-gateway', '^2.0.0') - // Detach is the single core disk-driven undo, resolved per client via its - // static descriptor (owning plugin + attach_probe) — not a per-adapter - // detach() hook and not the live gateway registry, so it reverses an on-disk - // attach even for a client whose adapter has been dropped/unloaded (LLP 0045 - // §Part 3). Build the bundled+installed descriptor map once and resolve from - // it; attach still needs the live adapter, so it resolves from the gateway. - const clientDescriptors = action === 'detach' ? await buildClientDescriptorMap(ctx) : undefined - - const clientNames = action === 'detach' - ? expandDetachClientNames(parsed.client, clientDescriptors) - : expandClientName(parsed.client, gateway) + const clientNames = expandClientName(parsed.client, gateway) if (clientNames.length === 0) { - const known = action === 'detach' - ? [...(clientDescriptors?.keys() ?? [])] - : gateway.listClients().map((c) => c.name) + const known = gateway.listClients().map((c) => c.name) ctx.stderr.write( - `error: unknown client '${parsed.client}'. ${ - action === 'detach' ? 'Known' : 'Registered' - } clients: ${known.join(', ') || '(none)'}\n` + `error: unknown client '${parsed.client}'. Registered clients: ${known.join(', ') || '(none)'}\n` ) return 1 } @@ -3283,56 +3314,40 @@ async function runClientLifecycle(action, argv, ctx) { let exitCode = 0 for (const name of clientNames) { try { - if (action === 'attach') { - const client = gateway.getClient(name) - if (!client) { - ctx.stderr.write(`error: unknown client '${name}'\n`) - exitCode = 1 - continue - } - // In dry-run mode the gateway source may not be started yet, - // so `localEndpoint()` could throw. Fall back to a placeholder - // endpoint — adapters are expected to short-circuit before - // touching it. - let endpoint - if (parsed.dryRun) { - try { - endpoint = gateway.localEndpoint() - } catch { - endpoint = configuredGatewayEndpoint(ctx.config) ?? 'http://127.0.0.1:0' - } - } else { - try { - endpoint = gateway.localEndpoint() - } catch (err) { - const configured = configuredGatewayEndpoint(ctx.config) - if (!configured) throw err - endpoint = configured - } + const client = gateway.getClient(name) + if (!client) { + ctx.stderr.write(`error: unknown client '${name}'\n`) + exitCode = 1 + continue + } + // In dry-run mode the gateway source may not be started yet, + // so `localEndpoint()` could throw. Fall back to a placeholder + // endpoint — adapters are expected to short-circuit before + // touching it. + let endpoint + if (parsed.dryRun) { + try { + endpoint = gateway.localEndpoint() + } catch { + endpoint = configuredGatewayEndpoint(ctx.config) ?? 'http://127.0.0.1:0' } - await client.attach({ - endpoint, - config: {}, - stdout: ctx.stdout, - stderr: ctx.stderr, - dryRun: parsed.dryRun, - json: parsed.json, - }) } else { - const descriptor = clientDescriptors?.get(name) - if (!descriptor) { - ctx.stderr.write(`error: unknown client '${name}'\n`) - exitCode = 1 - continue + try { + endpoint = gateway.localEndpoint() + } catch (err) { + const configured = configuredGatewayEndpoint(ctx.config) + if (!configured) throw err + endpoint = configured } - await detachClientViaCore({ - name, - descriptor, - dryRun: parsed.dryRun, - json: parsed.json, - ctx, - }) } + await client.attach({ + endpoint, + config: {}, + stdout: ctx.stdout, + stderr: ctx.stderr, + dryRun: parsed.dryRun, + json: parsed.json, + }) } catch (err) { const message = err instanceof Error ? err.message : String(err) ctx.stderr.write(`error: ${action} client '${name}' failed: ${message}\n`) @@ -3709,9 +3724,9 @@ async function runAgentsInstall(argv, ctx) { async function buildClientDescriptorMap(ctx) { /** @type {Map} */ const map = new Map() - /** @type {import('../manifest.js').LoadedManifest[]} */ + /** @type {LoadedManifest[]} */ let bundledLoaded = [] - /** @type {import('../manifest.js').LoadedManifest[]} */ + /** @type {LoadedManifest[]} */ let installedLoaded = [] try { const bundled = await discoverBundledPlugins() diff --git a/src/core/config/client_detach_disk.js b/src/core/config/client_detach_disk.js index 2ec2252..6bcf918 100644 --- a/src/core/config/client_detach_disk.js +++ b/src/core/config/client_detach_disk.js @@ -679,6 +679,18 @@ async function readJson(settingsPath, fs) { * @returns {Promise<{ content: string, existed: boolean, mtimeMs: number | undefined }>} */ async function readText(settingsPath, fs) { + // Stat BEFORE reading the content so the captured mtime never post-dates the + // bytes we return. If we stat'd after the read, a concurrent edit landing in + // the read→stat window would leave us holding stale content paired with the + // *new* mtime — and the write-time guard would then pass and silently clobber + // that edit. Stat-first instead makes the guard err toward CONCURRENT_EDIT. + let stat + try { + stat = await fs.stat(settingsPath) + } catch (err) { + if (errCode(err) === 'ENOENT') return { content: '', existed: false, mtimeMs: undefined } + throw new ClientDetachError(`failed to stat ${settingsPath}: ${errMsg(err)}`, { cause: err }) + } /** @type {string} */ let raw try { @@ -687,12 +699,6 @@ async function readText(settingsPath, fs) { if (errCode(err) === 'ENOENT') return { content: '', existed: false, mtimeMs: undefined } throw new ClientDetachError(`failed to read ${settingsPath}: ${errMsg(err)}`, { cause: err }) } - let stat - try { - stat = await fs.stat(settingsPath) - } catch (err) { - throw new ClientDetachError(`failed to stat ${settingsPath}: ${errMsg(err)}`, { cause: err }) - } return { content: raw, existed: true, mtimeMs: stat.mtimeMs } } diff --git a/src/core/daemon/runtime.js b/src/core/daemon/runtime.js index 8ea1b15..7d3c404 100644 --- a/src/core/daemon/runtime.js +++ b/src/core/daemon/runtime.js @@ -339,6 +339,13 @@ export async function runDaemon(opts = {}) { // configured-`listen` fallback the CLI uses. // All three stay undefined on a non-gateway boot, leaving the attach handler // inert by construction. + // + // Resolved ONCE here and then closed over by `runReconcilePass` below: the + // same `clientSeam` is reused, unchanged, for every reconcile pass for the + // daemon's lifetime — it is never re-derived per pass. So a pass can never + // observe a half-resolved seam (e.g. a transiently-empty `clients`); the + // attach handler's `desired()` always reads the fully-resolved-at-boot value, + // and reversal can never over-fire on a momentary `clients` gap. // @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — daemon resolves clientDescriptors from the catalog, clients/endpoint from boot.runtime.capabilities when the gateway is enabled const clientSeam = resolveClientActionSeam({ boot, fileLog }) diff --git a/test/core/integration.test.js b/test/core/integration.test.js index 51684b6..16a54e3 100644 --- a/test/core/integration.test.js +++ b/test/core/integration.test.js @@ -89,6 +89,19 @@ function fakeClientKernel({ clientNames = ['claude', 'codex'] } = {}) { return { registry, kernel, calls } } +/** + * A kernel with NO `hypaware.ai-gateway` capability — models the gateway + * plugin being uninstalled/unloaded. Detach must still run (it is the + * disk-driven core undo resolved from the static descriptor map, LLP 0045 + * §Part 3); attach stays gated and fails cap_missing. + */ +function fakeKernelWithoutGateway() { + const registry = createCommandRegistry() + registerCoreCommands(registry) + const kernel = createKernelRuntime({ commandRegistry: registry }) + return { registry, kernel } +} + /** @returns {Promise} */ async function freshHome() { return fs.mkdtemp(path.join(os.tmpdir(), 'hyp-integration-')) @@ -213,6 +226,48 @@ test('detach resolves an INSTALLED (non-bundled) client adapter from the bundled assert.equal(result.changed, false) }) +test('detach works with the @hypaware/ai-gateway capability absent (disk-driven undo, LLP 0045 §Part 3)', async () => { + // The gateway plugin is not installed/loaded, so the capability is absent. + // Detach is a pure on-disk undo resolved from the static descriptor map, so + // it must still succeed — it is NOT gated on the live gateway. (Attach stays + // gated; the next test proves it still fails cap_missing.) + const { registry, kernel } = fakeKernelWithoutGateway() + const home = await freshHome() + const result = await detach('codex', { + hypHome: home, + env: { ...process.env, HOME: home, CODEX_HOME: home }, + // @ts-expect-error test-only kernel injection + registry, + kernel, + }) + // Resolved 'codex' from the bundled descriptor map and ran the disk undo to a + // clean no-op (no marker on the temp tree) — not a cap_missing failure. + assert.equal(result.status, 'ok') + assert.equal(result.action, 'detach') + assert.equal(result.client, 'codex') + assert.equal(result.changed, false) +}) + +test('attach stays gated on the @hypaware/ai-gateway capability (cap_missing)', async () => { + // The counterpart to the detach-without-gateway test: attach genuinely needs + // the live adapter, so with the capability absent it must fail cap_missing + // rather than silently no-op. + const { registry, kernel } = fakeKernelWithoutGateway() + const home = await freshHome() + const result = await run(['attach', 'codex', '--json'], { + hypHome: home, + env: { ...process.env, HOME: home, CODEX_HOME: home }, + // @ts-expect-error test-only kernel injection + registry, + kernel, + }) + assert.equal(result.code, 1) + const json = /** @type {any} */ (result.json) + assert.equal(json.status, 'failed') + assert.equal(json.action, 'attach') + assert.equal(json.error_kind, 'cap_missing') +}) + test('attach throws HypAwareCommandError for an unknown client', async () => { const { registry, kernel } = fakeClientKernel() const hypHome = await freshHome() From cdab0e52cecbdf32caa9e23ab366a2656240c4a1 Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Fri, 26 Jun 2026 21:43:09 -0700 Subject: [PATCH 14/15] client-attach: unlink temp file on partial write; daemon auto-attach requires a bound endpoint (#179 round-3 hardening) Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0178wfuBrVQHwPbsHAtcE7YS --- llp/0045-client-attach.design.md | 10 ++- src/core/config/client_detach_disk.js | 37 ++++++--- src/core/config/gateway_endpoint.js | 12 +-- src/core/daemon/runtime.js | 36 +++++--- test/core/client-detach-disk.test.js | 55 +++++++++++++ test/core/daemon-attach-seam.test.js | 114 ++++++++++++++++++++++++++ 6 files changed, 232 insertions(+), 32 deletions(-) create mode 100644 test/core/daemon-attach-seam.test.js diff --git a/llp/0045-client-attach.design.md b/llp/0045-client-attach.design.md index e10ded8..5cad064 100644 --- a/llp/0045-client-attach.design.md +++ b/llp/0045-client-attach.design.md @@ -125,8 +125,14 @@ The daemon (`runDaemon` in [`src/core/daemon/runtime.js`](../src/core/daemon/run resolves all three from boot: `clientDescriptors` from the plugin catalog, `clients` from `boot.runtime.capabilities` when the gateway plugin is enabled (`capabilities.has('hypaware.ai-gateway', '^2.0.0')` guards the lookup), and -`endpoint` from `gateway.localEndpoint()` with the configured-`listen` fallback -the CLI already uses (`configuredGatewayEndpoint`). A client adapter plugin +`endpoint` from `gateway.localEndpoint()` — a **proven-bound** URL. (Hardening, +#179 round-3: the daemon path takes `localEndpoint()` and *only* that. If it +throws — the gateway never bound — the daemon leaves `endpoint` undefined and the +attach handler stays inert this pass rather than recording a base URL for a port +nothing bound; it attaches once a later boot observes a bound gateway. The +configured-`listen` fallback (`configuredGatewayEndpoint`) is kept only for the +**manual** `hyp attach`/`init` paths, where the user asked explicitly.) A client +adapter plugin *requires* the gateway capability ([LLP 0016](./0016-ai-gateway.decision.md)), so whenever a client plugin is enabled the gateway is too and the client is registered; `desired()` still guards on `ctx.clients.getClient(name)` being diff --git a/src/core/config/client_detach_disk.js b/src/core/config/client_detach_disk.js index 6bcf918..e580a54 100644 --- a/src/core/config/client_detach_disk.js +++ b/src/core/config/client_detach_disk.js @@ -746,20 +746,33 @@ async function writeTextAtomic(filePath, body, expectedMtimeMs, fs) { await fs.mkdir(path.dirname(filePath), { recursive: true }) const tmpPath = `${filePath}.${process.pid}.${crypto.randomBytes(6).toString('hex')}.tmp` - /** @type {FileHandle | undefined} */ - let handle - try { - handle = await fs.open(tmpPath, 'w', 0o600) - await handle.writeFile(body, 'utf8') - await handle.sync() - } finally { - if (handle) await handle.close() - } + // The uniquely-named temp file must never outlive a *failed* write: a + // write/sync/close error before the final rename would otherwise orphan it on + // disk (these names are unique per call, so a leak accumulates rather than + // being overwritten on retry). Track whether the rename committed and + // best-effort unlink the temp on every other exit — swallowing the unlink + // error so a cleanup hiccup never masks the real write/rename failure. + let renamed = false try { + /** @type {FileHandle | undefined} */ + let handle + try { + handle = await fs.open(tmpPath, 'w', 0o600) + await handle.writeFile(body, 'utf8') + await handle.sync() + } finally { + if (handle) await handle.close() + } await fs.rename(tmpPath, filePath) - } catch (err) { - await fs.rm(tmpPath, { force: true }) - throw err + renamed = true + } finally { + if (!renamed) { + try { + await fs.rm(tmpPath, { force: true }) + } catch { + // Best-effort: a leaked temp file is preferable to losing the original error. + } + } } } diff --git a/src/core/config/gateway_endpoint.js b/src/core/config/gateway_endpoint.js index 7a1dcc6..4cdbf0f 100644 --- a/src/core/config/gateway_endpoint.js +++ b/src/core/config/gateway_endpoint.js @@ -6,11 +6,13 @@ /** * Resolve the gateway endpoint from the active config's `@hypaware/ai-gateway` - * `listen` directive, for callers that need the URL before the gateway source - * is live in this process (`hyp attach`, the `init` walkthrough, and the - * daemon's attach reconciler when `localEndpoint()` is not yet bindable). It is - * the configured-`listen` fallback shared by all three so they can never derive - * a different port from the same config. + * `listen` directive, for **manual** callers that need the URL before the + * gateway source is live in this process (`hyp attach` and the `init` + * walkthrough). It is the configured-`listen` fallback shared by both so they + * can never derive a different port from the same config. The daemon's + * auto-attach path deliberately does *not* use this fallback — involuntary + * attach requires a proven-bound `localEndpoint()` so it never records a URL for + * a port nothing bound (see `resolveClientActionSeam` in `daemon/runtime.js`). * * Returns `undefined` when the gateway plugin is absent, its config is not an * object, or `listen` is missing/malformed — the caller then falls back to a diff --git a/src/core/daemon/runtime.js b/src/core/daemon/runtime.js index 7d3c404..d059c49 100644 --- a/src/core/daemon/runtime.js +++ b/src/core/daemon/runtime.js @@ -16,7 +16,6 @@ import { buildConfigApplyDeps } from '../config/apply_deps.js' import { createActionReconciler } from '../config/action_reconciler.js' import { attachHandler } from '../config/action_attach.js' import { backfillHandler } from '../config/action_backfill.js' -import { configuredGatewayEndpoint } from '../config/gateway_endpoint.js' import { bootKernel, resolveLayeredConfigForDaemon } from '../runtime/boot.js' import { createSinkDriver } from '../sinks/driver.js' import { materializeSinks } from '../sinks/materialize.js' @@ -335,8 +334,9 @@ export async function runDaemon(opts = {}) { // plugins (the static catalog the boot already built); // - `clients` is the runtime gateway capability used only to invoke a // client's attach effect, present only when the gateway plugin is enabled; - // - `endpoint` is the local gateway base URL, from `localEndpoint()` with the - // configured-`listen` fallback the CLI uses. + // - `endpoint` is the proven-bound local gateway base URL from + // `localEndpoint()` (no configured-`listen` fallback on the daemon path — + // auto-attach must never record a URL for a port nothing bound). // All three stay undefined on a non-gateway boot, leaving the attach handler // inert by construction. // @@ -813,15 +813,21 @@ export function createReconcilePassScheduler({ run, log }) { * too; on a non-gateway boot `clients`/`endpoint` stay undefined and the attach * handler is inert by construction. * - * `endpoint` prefers the live `localEndpoint()` (the gateway source is already - * bound by the time the reconciler is constructed — `startConfiguredSources` - * ran during boot) and falls back to the configured-`listen` URL, the same - * fallback `hyp attach` uses, so a not-yet-bound gateway still points clients at - * the right port. + * `endpoint` is the live `localEndpoint()` and *only* that — a **proven-bound** + * gateway URL. The gateway source is already bound by the time the reconciler is + * constructed (`startConfiguredSources` ran during boot), so `localEndpoint()` + * returns the real bound port. If it throws — the gateway never bound (e.g. its + * listen failed) — the daemon must **not** fall back to the configured-`listen` + * URL: auto-attach is involuntary, and recording a base URL for a port nothing + * bound would point clients at a dead endpoint. Instead `endpoint` stays + * undefined and the attach handler's `perform()` guard keeps it inert this pass + * (attaching once the gateway is proven-bound on a later boot). Manual + * `hyp attach`/`init` keep the configured-`listen` fallback — there the user + * asked explicitly (`core_commands.js`). * * @param {{ boot: BootKernelResult, fileLog: ReturnType }} args * @returns {{ clientDescriptors: Map, clients: AiGatewayCapability | undefined, endpoint: string | undefined }} - * @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — clientDescriptors from the catalog; clients/endpoint from boot.runtime.capabilities, guarded on the gateway capability + * @ref LLP 0045#part-1--the-client-seam-in-the-reconcile-context [implements] — clientDescriptors from the catalog; clients/endpoint from boot.runtime.capabilities, guarded on the gateway capability; daemon endpoint requires a proven-bound localEndpoint() (no configured-listen fallback — that's the manual path's) */ function resolveClientActionSeam({ boot, fileLog }) { const clientDescriptors = boot.clientDescriptors @@ -837,13 +843,16 @@ function resolveClientActionSeam({ boot, fileLog }) { try { endpoint = clients.localEndpoint() } catch { - // The gateway source may not be bound yet (e.g. its listen failed); fall - // back to the configured port so attach still writes the right URL. - endpoint = boot.config ? configuredGatewayEndpoint(boot.config) : undefined + // The gateway never bound (e.g. its listen failed). Unlike manual + // `hyp attach`, the daemon does NOT fall back to the configured-`listen` + // URL — auto-attach must never record a base URL for an unbound port. + // Leave `endpoint` undefined; the handler stays inert until a later boot + // observes a proven-bound gateway. + endpoint = undefined } if (!endpoint) { fileLog.warn('daemon.attach_endpoint_unresolved', { - hyp_reason: 'no_local_endpoint_and_no_configured_listen', + hyp_reason: 'no_bound_local_endpoint', }) } } @@ -1077,4 +1086,5 @@ function sleep(ms) { export { pidFilePath, statusFilePath, + resolveClientActionSeam, } diff --git a/test/core/client-detach-disk.test.js b/test/core/client-detach-disk.test.js index 4bed057..5feca9d 100644 --- a/test/core/client-detach-disk.test.js +++ b/test/core/client-detach-disk.test.js @@ -439,3 +439,58 @@ test('undo is a no-op for a descriptor without an attachProbe', async () => { }) assert.equal(result.changed, false) }) + +/* ------------------------- atomic-write temp cleanup ------------------------- */ + +/** + * An `fs` double that delegates to the real `node:fs/promises` for everything + * except the temp-file handle's `sync()`, which throws — simulating a + * write/fsync failure *after* the uniquely-named temp file is created but + * *before* the final rename. Used to prove the atomic writer never orphans the + * temp file on a partial write. + * @returns {any} + */ +function makeSyncFailingFs() { + return /** @type {any} */ ({ + stat: (/** @type {string} */ p) => fs.stat(p), + readFile: (/** @type {string} */ p, /** @type {any} */ enc) => fs.readFile(p, enc), + mkdir: (/** @type {string} */ p, /** @type {any} */ opts) => fs.mkdir(p, opts), + rename: (/** @type {string} */ a, /** @type {string} */ b) => fs.rename(a, b), + rm: (/** @type {string} */ p, /** @type {any} */ opts) => fs.rm(p, opts), + async open(/** @type {string} */ p, /** @type {any} */ flags, /** @type {any} */ mode) { + const handle = await fs.open(p, flags, mode) + return { + writeFile: (/** @type {any} */ data, /** @type {any} */ enc) => handle.writeFile(data, enc), + sync: async () => { throw new Error('boom: simulated fsync failure') }, + close: () => handle.close(), + } + }, + }) +} + +test('the atomic write unlinks the temp file on a partial write — no orphaned .tmp', async () => { + const home = await stageHome() + try { + const original = { env: { ANTHROPIC_API_KEY: 'sk-x', ANTHROPIC_BASE_URL: 'https://foreign.example/api' } } + const settingsPath = await writeClaudeSettings(home, JSON.stringify(original, null, 2) + '\n') + // Real self-describing marker, so the undo proceeds all the way to the write. + await claudeAttach({ ...ATTACH, settingsPath }) + + const dir = path.dirname(settingsPath) + const before = (await fs.readdir(dir)).sort() + + // The injected fs fails the fsync — after the temp file exists, before rename. + await assert.rejects( + detachClientFromDisk({ descriptor: CLAUDE_DESCRIPTOR, homeDir: home, fs: makeSyncFailingFs() }), + /simulated fsync failure/ + ) + + const after = (await fs.readdir(dir)).sort() + // No uniquely-named temp file left behind by the failed write. + assert.equal(after.some((e) => e.endsWith('.tmp')), false, `orphaned tmp files: ${after.join(', ')}`) + // The rename never ran, so the directory is exactly as it was pre-write. + assert.deepEqual(after, before) + } finally { + await fs.rm(home, { recursive: true, force: true }) + } +}) diff --git a/test/core/daemon-attach-seam.test.js b/test/core/daemon-attach-seam.test.js new file mode 100644 index 0000000..117ed6f --- /dev/null +++ b/test/core/daemon-attach-seam.test.js @@ -0,0 +1,114 @@ +// @ts-check + +import test from 'node:test' +import assert from 'node:assert/strict' + +import { resolveClientActionSeam } from '../../src/core/daemon/runtime.js' + +/** + * #179 round-3 hardening (LLP 0045 §Part 1): the daemon's client-action seam + * resolves the auto-attach `endpoint` from a *proven-bound* gateway only. When + * `localEndpoint()` throws (the gateway never bound) the daemon must NOT fall + * back to the configured-`listen` URL — recording a base URL for a port nothing + * bound would point clients at a dead endpoint. The manual `hyp attach`/`init` + * paths keep that fallback (`core_commands.js`); this is the daemon path only. + */ + +/** A quiet file log that records its `warn` calls. */ +function makeFileLog() { + /** @type {Array<{ event: string, attrs: any }>} */ + const warnings = [] + return { + warnings, + warn(/** @type {string} */ event, /** @type {any} */ attrs) { warnings.push({ event, attrs }) }, + info() {}, + error() {}, + debug() {}, + } +} + +/** + * A minimal `BootKernelResult` double exposing just what the seam reads: the + * ai-gateway capability (with a stubbed `localEndpoint`) and a config whose + * configured `listen` *would* resolve — so the unbound case proves the daemon no + * longer falls back to it. + * @param {{ localEndpoint: () => string }} opts + * @returns {any} + */ +function makeBoot({ localEndpoint }) { + const capability = { + localEndpoint, + getClient() { return undefined }, + listClients() { return [] }, + registerClient() {}, + registerUpstreamPreset() {}, + registerExchangeProjector() {}, + registerSettlementEnricher() {}, + } + return { + clientDescriptors: new Map(), + // Configured listen resolves to http://127.0.0.1:8787 — the *old* fallback + // target. The daemon path must ignore it when localEndpoint() throws. + config: { version: 2, plugins: [{ name: '@hypaware/ai-gateway', config: { listen: '127.0.0.1:8787' } }] }, + runtime: { + capabilities: { + has: (/** @type {string} */ name) => name === 'hypaware.ai-gateway', + require: () => capability, + }, + }, + } +} + +test('seam records the proven-bound localEndpoint as the daemon attach endpoint', () => { + const fileLog = makeFileLog() + const seam = resolveClientActionSeam({ + boot: makeBoot({ localEndpoint: () => 'http://127.0.0.1:54321' }), + fileLog: /** @type {any} */ (fileLog), + }) + assert.equal(seam.endpoint, 'http://127.0.0.1:54321') + assert.notEqual(seam.clients, undefined) + assert.equal(fileLog.warnings.length, 0, 'no unresolved-endpoint warning when bound') +}) + +test('seam does NOT fall back to the configured listen when localEndpoint() throws (no URL for an unbound port)', () => { + const fileLog = makeFileLog() + const seam = resolveClientActionSeam({ + boot: makeBoot({ + localEndpoint: () => { + throw new Error('ai-gateway: localEndpoint() called before the gateway started') + }, + }), + fileLog: /** @type {any} */ (fileLog), + }) + // The configured listen (127.0.0.1:8787) WOULD have resolved — the daemon path + // must still leave endpoint undefined so auto-attach stays inert this pass. + assert.equal(seam.endpoint, undefined) + // The gateway capability itself is still present (clients can be invoked once + // a later boot observes a bound gateway). + assert.notEqual(seam.clients, undefined) + // It surfaces the unresolved-endpoint warning for operators. + assert.equal( + fileLog.warnings.some((w) => w.event === 'daemon.attach_endpoint_unresolved'), + true, + 'expected a daemon.attach_endpoint_unresolved warning' + ) +}) + +test('seam is inert (no clients/endpoint) when the ai-gateway capability is absent', () => { + const fileLog = makeFileLog() + /** @type {any} */ + const boot = { + clientDescriptors: new Map(), + config: { version: 2, plugins: [] }, + runtime: { + capabilities: { + has: () => false, + require() { throw new Error('require() must not be called without the capability') }, + }, + }, + } + const seam = resolveClientActionSeam({ boot, fileLog: /** @type {any} */ (fileLog) }) + assert.equal(seam.clients, undefined) + assert.equal(seam.endpoint, undefined) + assert.equal(fileLog.warnings.length, 0) +}) From d421761e94f232e53dd62fd7b0e3ca72bd282d9b Mon Sep 17 00:00:00 2001 From: Phillip Cunliffe Date: Mon, 29 Jun 2026 15:17:35 -0700 Subject: [PATCH 15/15] Fix build:types: import ClientDescriptor from the src/core/types barrel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master's PR #196 relocated ClientDescriptor into the src/core/types.d.ts barrel. The merge left four files still importing it from src/core/plugin_catalog.js, which itself only @imports the type (for local use) and does not re-export it → TS2459 in `npm run build:types`, run via `prepare` on `npm i`. That broke all three CI jobs (lint/typecheck/test) at the install step before any tests ran, while the resolver's local `npm test` passed because its symlinked node_modules skipped prepare. Repoint all four to the canonical barrel, matching sibling files (config/validate.js, cli/walkthrough.js): root-anchored `src/core/types.js` for the emitted .js files, `.d.ts` for the input config/types.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/config/client_detach_disk.js | 2 +- src/core/config/types.d.ts | 2 +- test/core/action-attach.test.js | 2 +- test/core/client-detach-disk.test.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/config/client_detach_disk.js b/src/core/config/client_detach_disk.js index e580a54..367acee 100644 --- a/src/core/config/client_detach_disk.js +++ b/src/core/config/client_detach_disk.js @@ -9,7 +9,7 @@ import { resolveClientSettingsPath } from '../daemon/client_settings_path.js' /** * @import { FileHandle } from 'node:fs/promises' - * @import { ClientDescriptor } from '../plugin_catalog.js' + * @import { ClientDescriptor } from '../../../src/core/types.js' * @import { DetachFromDiskResult } from './types.d.ts' */ diff --git a/src/core/config/types.d.ts b/src/core/config/types.d.ts index 4ff02c1..e55d07e 100644 --- a/src/core/config/types.d.ts +++ b/src/core/config/types.d.ts @@ -13,7 +13,7 @@ import type { JsonObject, AiGatewayCapability, } from '../../../collectivus-plugin-kernel-types.d.ts' -import type { ClientDescriptor } from '../plugin_catalog.js' +import type { ClientDescriptor } from '../../../src/core/types.d.ts' /** * Outcome of the `init` overwrite guard (LLP 0031). `proceed` is true diff --git a/test/core/action-attach.test.js b/test/core/action-attach.test.js index ce7c63a..6758970 100644 --- a/test/core/action-attach.test.js +++ b/test/core/action-attach.test.js @@ -17,7 +17,7 @@ import { detachClientFromDisk } from '../../src/core/config/client_detach_disk.j * the reconciler's generic gap loop is covered by `action-reconciler.test.js`. * * @import { ActionContext, ActionHandler } from '../../src/core/config/types.d.ts' - * @import { ClientDescriptor } from '../../src/core/plugin_catalog.js' + * @import { ClientDescriptor } from '../../src/core/types.js' */ /** diff --git a/test/core/client-detach-disk.test.js b/test/core/client-detach-disk.test.js index 5feca9d..b995dfa 100644 --- a/test/core/client-detach-disk.test.js +++ b/test/core/client-detach-disk.test.js @@ -22,7 +22,7 @@ import { prepareAttach as codexPrepareAttach } from '../../hypaware-core/plugins * loaded** at reverse time, proving it never depends on `ctx.clients`. */ -/** @import { ClientDescriptor } from '../../src/core/plugin_catalog.js' */ +/** @import { ClientDescriptor } from '../../src/core/types.js' */ /** @type {ClientDescriptor} */ const CLAUDE_DESCRIPTOR = {